3 ms·

So the two cases against writing the most legible, succinct code given the specifications at the time of writing it are: >Firstly, I didn’t talk to the person

So the two cases against writing the most legible, succinct code given the specifications at the time of writing it are: >Firstly, I didn’t talk to the person who wrote it. I rewrote the code and checked it in without their input. Even if it was an improvement (which I don’t believe anymore), this is a terrible way to go about it. A healthy engineering team is constantly building trust. Rewriting your teammate’s code without a discussion is a huge blow to your ability to effectively collaborate on a codebase together. There's no question about this. Nobody likes the self-proclaimed savant who works in isolation and makes sweeping changes to the codebase or other people's work without collaborating and gaining some consensus. If it's a change worth making it should be a simple case to present to your (hopefully) equally intelligent team. There is a difference, it has to be highlighted, between refactoring someone's code in order to extend it yourself and simply re-writing someone's implementation because it doesn't suit your requirements. The former is part of the job, the latter should at the very least be an opportunity to mentor the person's whose code you want to re-write in why it was suboptimal and guide them on the changes you'd like to make, or even give them the chance to make it themselves. This is kind of what code reviews are supposed to do. That does not negate the need to structure and optimize code to remove duplication whatsoever. It's not an argument against clean code standards and it's weak that it amounts to 50% of his case here. >Secondly, nothing is free. My code traded the ability to change requirements for reduced duplication, and it was not a good trade. For example, we later needed many special cases and behaviors for different handles on different shapes. My abstraction would have to become several times more convoluted to afford that, whereas with the original “messy” version such changes stayed easy as cake. Time changes, requirements change. It's part and parcel of our jobs in software development. Writing code that at one point is optimal and most legible for the cases present should also be done to try to make it refactorable and extendable. It is much easier to refactor and extend code that isn't riddled with duplication and mangled with hardcoded business logic. Abstract your code and write your implementations well, name things in a way that people can read it and write tests that describe what's expected from it. Refactoring well isn't easy work. Refactoring a sprawling legacy codebase with a lot of duplication and legibility problems is significantly worse. I'm not saying we need to be dogmatic here. If you're given the opportunity to develop new code you should be aiming to do the best job of it given what you know now, in a way that will be comprehensible to you, or whoever needs to touch that code next. We all know that there are problems with premature optimization caused by "best practices" evangelists who'd happily drive up time-to-market and operating costs/complexity exponentially in the name of having the codebase and applications / services architecture in line with whatever he or she has read lately from "thought leaders" in our industry, but writing the code for a given application in line with the above isn't one of them.

scarface74 7y ago
It is much easier to refactor and extend code that isn't riddled with duplication and mangled with hardcoded business logic. Abstract your code and write your implementations well, name things in a way that people can read it and write tests that describe what's expected from it. Especially in a statically compiled language - “extract method”, “extract class”, “pull members up”, etc. is an automated, guaranteed safe refactor (ignoring reflection) .
mntmoss 7y ago
The code is not large enough to need maintenance at a fine-grained level. There is a secondary rule to the DRY "rule of three": If I can blow it away and rewrite it so easily, there is nothing to reuse or refactor in it. The feature is done, and we are into code golf and speculation, neither of which are productive uses of time. In my experience the success rate of speculative refactors like the one author made has perhaps a 50/50 chance, so no better than the initial strategy. It's the requirements themselves and the application of techniques to avoid various classes of errors that give code direction and structure - not the aesthetics at a moment in time(which is what author took issue with). If you spot multiple approaches on the first try, you can add a comment with a date outlining alternatives so that the conversation may be resumed later when the new requirements come in. But at all times you're always at the mercy of "discipline", and there's no preemptive measure that avoids that.
- Rapzid 7y ago
  The author also didn't sound like a particularly senior engineer at the time for many reasons. So the original code author and the "boss" may have been taking into consideration timelines and future work/requirements coming down the pipe. A very valid reason could have been as simple as "We are re-visiting this in a couple sprints after feedback and will have a better idea of how it needs to change. The extra day spent on this wasn't worth pushing getting it into peoples hands, and we don't know if it would be a waste." The author would have known this if he started a conversation about it.
  - cc81 7y ago

scarface74 7y ago

It is much easier to refactor and extend code that isn't riddled with duplication and mangled with hardcoded business logic. Abstract your code and write your implementations well, name things in a way that people can read it and write tests that describe what's expected from it. Especially in a statically compiled language - “extract method”, “extract class”, “pull members up”, etc. is an automated, guaranteed safe refactor (ignoring reflection) .

mntmoss 7y ago

The code is not large enough to need maintenance at a fine-grained level. There is a secondary rule to the DRY "rule of three": If I can blow it away and rewrite it so easily, there is nothing to reuse or refactor in it. The feature is done, and we are into code golf and speculation, neither of which are productive uses of time. In my experience the success rate of speculative refactors like the one author made has perhaps a 50/50 chance, so no better than the initial strategy. It's the requirements themselves and the application of techniques to avoid various classes of errors that give code direction and structure - not the aesthetics at a moment in time(which is what author took issue with). If you spot multiple approaches on the first try, you can add a comment with a date outlining alternatives so that the conversation may be resumed later when the new requirements come in. But at all times you're always at the mercy of "discipline", and there's no preemptive measure that avoids that.

Rapzid 7y ago
The author also didn't sound like a particularly senior engineer at the time for many reasons. So the original code author and the "boss" may have been taking into consideration timelines and future work/requirements coming down the pipe. A very valid reason could have been as simple as "We are re-visiting this in a couple sprints after feedback and will have a better idea of how it needs to change. The extra day spent on this wasn't worth pushing getting it into peoples hands, and we don't know if it would be a waste." The author would have known this if he started a conversation about it.
- cc81 7y ago