7 ms·
Always leave the code better than you found it
- meowster 6y agoOff topic, but the title reminds me of Scouts: Always leave a place cleaner than you found it. I think it's a good philosophy, whether it's about code or trash. It makes everyone's life better overall.
- casion 6y agoUntil you make a bunch of choices about "better" that are based on presumptions that are incorrect. Now you're just making things worse and filling yourself with a sense of accomplishment that puts you out of touch with reality. When I was in scouts, our scout leader was adamant that we should leave the place "The way it would have been if left alone" and not try to push our own ideas of "better" on things. Which I think is a much better line of thinking, even if still flawed.
- joncampbelldev 6y agoWhilst improvements shouldn't be done ad-hoc without consideration, I think the concept of "left alone" applies more easily to the concept of natural spaces. In code it is really stretching the metaphor to do much more than very basic tasks like renaming or code formatting (e.g. picking up trash). Adding a test doesn't feel like "left alone". One of the examples in the article is upgrading a dependency. This could easily fall into the category you mention if the upgrade itself is not properly tested. A considered approach would be: "I want to make a small improvement, oh I see this dependency is out of date and has some minor vulnerabilities or perf issues, or is missing a feature that could come in handy for XYZ. Hmm, there are no tests confirming our assumptions about the current library version, nor are then any tests are our code / systems that use it. I guess my improvement should be to add those tests, then next time someone has the same idea it'll be an easier upgrade" I am unsure but I also interpret from your first sentence that you may have seen people use this rule as a defence for gold plating (the opposite of KISS or YAGNI). Similarly this should fall under the heading of changes made without proper discussion, thought or consideration. Otherwise I think we're really stuck, if team members cannot be trusted to properly interpret the boy scout rule without messing up a codebase then how on earth can they be trusted to build the foundation of their company / team / system?
- alisonkisk 6y ago"left alone" corresponds to how it was designed. Your task is to push the code a little closer to the design, which always has compromises in implementation. The big fights come in (as seen in comments in this post already) when there was no original design, and it's mess vs possibly dubious attempts to clean up the mess.
- joncampbelldev 6y agoI think another possibility in addition to what you mentioned is that the original design was incorrect, or even just not very optimal. Just as an aside. However this is all becoming a wide ranging statement about the entire job of a dev team. So far removed from the extremely innocuous advice of the article around "make small improvements regularly". Whilst you can apply caveats to that around "test your improvements", "check your improvement with others more familiar with that code" etc etc, these are all things that should be expected of a professional. At the risk of talking about no-true-scotsman. If someone cannot be trusted with the advice "make small improvements regularly" then what the f* can they be trusted with?
- paulintrognon 6y agoThe funny thing is, is leaving the land "the way it would have been if left alone" better? In many cases, it is debatable. In New Zealand, there are many invasive species of animals and trees. Should they interfere and try to remove the invasive species? Or you should they "leave it alone" and let the pine trees take over all native plant species? (the same is happening in the french Alpes where pine trees are invading a lot of land).
- cyphar 6y agoIn fairness, if the land had been left alone there wouldn't have been any invasive species.
- skrebbel 6y agoThat's why it's called "the boyscout rule" and the verb "to boyscout". Eg in a PR description "I also boyscouted the entire login service a bit while I was in there".
- bank90210 6y agoUgh...had a company consult for us who had this as their moto. A guy decided to refactor all the code the way he felt it should be. It had no concept of underlying logic and reason why previous decisions were made. It broke many systems and the guy was fired, but we were left with his crappy code.
- joncampbelldev 6y agoThis sounds very different to the process described in post. Are you raising this anecdote as an argument against the boy scout rule? I don't think you would find many people who would consider his actions a reasonable interpretation of it. I think this person would cause damage regardless of their motto or underlying intent. Taking a leaf from the guidelines for person-to-person,discussion on this forum about arguing against the strongest version of your opponents argument: I think there is great value to lots of small improvements over a long period of time (no silver bullet etc). Certainly it should be tempered with the knowledge that the change is indeed an improvement. That can usually only be decided by the person who originally wrote it OR a consesus of the team involved.
- dahfizz 6y agoThe problem is that there is no objective measure of "better". The consultant did think he was leaving the code better than he found it. I've worked on codebases where the whole team agrees a certain part of the codebase needs to be refactored, but we all also agree it is way too dangerous. Which is better? Working but hard to maintain spaghetti code, or clean, new, and broken code?
- joncampbelldev 6y agoSo for the consultant: Yes he did, clearly this consultant was an absolute disaster. An excellent cautionary tale of arrogance combined with lack of ability and design thinking. Changes made without thought / design or discussion with the team are unlikely to be "improvements", unless they're being made by someone intimately familar with the codebase (which an outside consultant cannot be). For your second point, I don't really see the applicability of this specific concept of the boy scout rule to the situation of "business critical code that is in such a bad state the whole team agrees it should be rewritten". However, were I to try to apply the concept to your situation, the first question to ask is "why are people afraid to change the code?". There's usually 2 reasons for this. Firstly, the code and the problem are super complex and hard to understand, this just takes a lot of time, to develop the knowledge and intuition around the code. Secondly, the code lacks tests (unit / intergration / functional / manual spreadsheet of test cases) that assert the behaviour you expect. Therefore my (unasked for and probably already known to you) suggestions for "leaving it cleaner than you found it" would be to make a start on understanding the code better through a series of very very very small refactorings. Pick one tiny function that is a mess and clean it up. At the same time, ensure your changes are correct by asserting all expected behaviour of that function. If you are now saying "duh of course" well thats the point. I do not think a single reasonable person would suggest that a full rewrite of a complex business critical system that is either poorly understood, poorly tested or both is a charitable interpretation of what the article is talking about. At the end of the day it is super easy to shoot down any idea or concept in programming, because we have so much power to abuse and ruin anything day to day if we're not careful. It's the easiest thing in the world to take a machete to a codebase without thought. But the problem there is the person holding the machete, they can't be trusted with it. I cannot think of a single thing that wouldn't be misinterpreted or done badly by some of the terrible coders I've met during my career.
- sys_64738 6y agoWho reviews code changes by the developer?
- joncampbelldev 6y agoOther developers, unless it's a solo dev at a small company. Then roll the dice and hope you hired someone with good design sensibilities.
- sys_64738 6y agoSo you’d hope this and the prior design review would stop the consultant from wasted effort. Probably not.
- joncampbelldev 6y agoI'm not entirely clear what you mean. Are you referring to the disaster artist consultant mentioned in another comment? I don't think anything can stop someone like that from wrecking things short of not hiring them. Or do you mean that small incremental improvements shouldn't be attempted because a proper review of whether they are worth it or not will be "wasted effort" if its decided they are not worth it? Or something else? Not being sassy, genuinely unsure. As for me personally, I would never hire an outside consultant to improve a codebase. The incentives are all wrong (long term maintenance to be done after they leave) and it's unlikely they'll be working on it for long enough to properly understand either the codebase or the domain.
- sys_64738 6y agoIt all goes back to what was the consultant actually hired to do? Was it to rewrite that code? Who is watching what the consultant actually does?
- joncampbelldev 6y agoPlease may you specify what consultant you're referring to here so I can answer more thoroughly. In general, I highly doubt anyone would hire a consultant just to "rewrite code". Rewriting code is not a business objective. Improving application performance is, delivering features more efficiently and reliably is. These are some examples of objectives that might involve rewriting code (or not, not every codebase is a swamp in need of draining). In the context of this article though, the code to be "improved" should be anything you touch. Did you use a function in building some new widget and you noticed it had no tests and no docs so it took you ages to integrate? Then add a test and the minimal doc string. In terms of who is watching the consultant, I'm unclear why you're asking me, a person who has just said they would never hire a consultant to do anything with code. But if I had to, I would get someone of at least equivalent technical knowledge. However I'm pretty sure thats not how it usually works, which is probably why inept consulting companies rinse big companies for so much money for such lacklustre results. But overall I really have no idea what you're talking about with regard to consultants and rewriting code. None of which seems to have much relevance to this article about making tiny improvements regularly. Unless your comment was in the wrong thread?
- umvi 6y agoGreat article. A lot of times even just the first one (Documentation) is immensely helpful. For example, one time I went through a comment-less source file full of mathematical formulas and replaced magic numbers with meaningful constants and documented which RF chipset datasheet each formula/constant was coming from (including page number). Other developers thanked me later when they were looking for the same thing.
- forest_dweller 6y agoIt really depends on the organisation. This sounds sensible, pro-active and productive until to work on legacy systems that have been in place probably or organisations that don't value any of these activities (you typically don't want to stay there but sometimes you just gotta put up with a role until you get a chance to move on). Generally in this scenario your job is to make a change that fulfils the requirements, verify that it works and test it manually, pass it onto QA and move on. Businesses often don't care your improvements unless it is a new feature and it will make you look bad because it will take you longer to complete a task than they deem necessary. As for refactoring in this situation. Don't. Even making seemly simple changes can it makes it difficult for others to code review it, or could incur breakage (yes there are some systems that are this fragile).
- tuwtuwtuwtuw 6y agoThe issue I run into with this comes part of the code review. So some developer is going to fix some code, and the code is ugly. So the developer fixes the bug and makes the code prettier and push it as a single commit. I am then left with figuring out which changes are related to the bug fix and what is related to the cleanup. If I tell the developer to first fix the bug and then cleanup in a separate commit, it may be inefficient since the fix after cleanup may not be the same as fix before cleanup. It may also lead to the cleanup being skipped because his work is technically done after the fix. If I tell him to cleanup first and fix later, it's also seem to lead to issues since the developer has typically already figured out how to fix it since he came into the code with the mindset of fixing the bug. Of course I like the code being cleaned up, and I like the idea that you improve the code as you work with it. But I'm not sure what the best method is. Maybe the issue is with me, that I want the cleanup and fix in separate commits for easier reviewing. Personally if I spot ugly code when fixing a bug I add unit tests, fix the bug and then add another ticket to improve the code. I then plan that code cleanup for this or the next sprint. But it feels tedious and is probably inefficient as well
- goalieca 6y ago> I am then left with figuring out which changes are related to the bug fix and what is related to the cleanup. Hmm. That has never been an issue for me in practice unless they go way too far with the refactoring. Usually it’s me who suggests cleanup for defensive coding reasons because the bug is often a coding issue.
- hinkley 6y agoPeople frustrate me by taking the short view of these things. Bigger refactorings and architectural changes usually require tracing the tendrils of an assumption out through the code. It’s not a matter of “can they figure it out,” it’s a matter of doing that in ten places and still somehow hold the other three bits of critical data in short term memory. If this path isn’t fruitful then there are five other theories to try so it’s not particularly amenable to writing it down. It’s flow state depth first search, and the crappy non-idiomatic code is ruining the flow. People always try to deflect based on some intelligence argument. I’m not trying to understand your code, I’m trying to understand something you missed. This isn’t about intelligence, it’s about wisdom.
- amw-zero 6y agoThere’s nothing wrong with optimism. But this line of thinking is a bit naive unfortunately. I’d say pretty much no one agrees on what “better” looks like. Better to someone often means applying a pattern that they just discovered in a blog post that week. Software design is still entirely subjective. For example, “better” once meant refactoring to be more object-oriented. That’s backfired a bit recently, so now “better” might mean having a more functional design. There’s no evidence that either of these actually have any impact on the end result of quality as perceived by users. Anyway, it’s good to be positive. There’s probably subconscious good that comes from that.
- tuwtuwtuwtuw 6y agoI think in a huge number of cases it is possible to objectively say that a change made the code better. I see those very often when consulting at companies. Just to give you an example (which I ran across last week), if you have some code handling a cart in an e-commerce site and you need to keep track of the number of items in this cart would you prefer the variable name "totalAmount" or "quantity". There's also a variable named "totalAmountIncludingTax" which holds the monetary amount to pay for the items in the cart.
- mattgreenrocks 6y agoThe accumulation of many small wins in things like naming is often much more helpful than full-on overhauls. But you have to be willing to do the small, seemingly inconsequential things to build up to those.
- amw-zero 6y agoHow do you know that one name is better than the other? I see renames all the time where I preferred the original version.
- tuwtuwtuwtuw 6y agoIf you have a variable representing the length of an array containing user accounts, would you prefer the name "length" or "price"?
- wufufufu 6y agoI applaud this, but I also advise against this for your career. Unless the change you're making has immediate and significant business benefits, from a PM/EM perspective you're 1) wasting time 2) potentially introducing bugs The correct move is to wait until the org is so bogged down with tech debt that meaningful progress cannot be made, then switch companies/teams.
- tuwtuwtuwtuw 6y agoJust to be clear, this is a joke right?
- wufufufu 6y agoI didn't intend it as a joke. Maybe at a developer-friendly company it could be viewed as a joke.
- burade 6y agoIt will only be joke once our profession unionizes. Until then, nope, not a joke. It's actually very good career advice.
- tuwtuwtuwtuw 6y agoI'm in a union but actually not sure how that is relevant. I'm in Sweden, maybe things are different. I have never worked with a PM who hasn't understood the concept of technical debt though. It sounds like a bad career move to work with such a person. Ignoring technical debt until development slows down to a crawl is a good idea? I don't buy it. My advice would be to either try to fix the issue (both the PM and the technical debt) immediately or find a new job.
- Viliam1234 6y agoIn my experience, managers do understand the concept of technical debt, in theory. They just don't think we should be actually addressing it right now, because something else is more urgent. And there is always something more urgent.
- sigsergv 6y agoThere are some very valid points there: improve documentation or test. But others are not: refactoring (this should be done explicitly, with proper testing and approvement from management), dependency upgrade (the same reasons).
- basicallydan 6y agoA very nice sentiment! I think one of the most useful parts of this list of suggestions is to write or improve tests. You did allude to this, but I think it’s the most useful part (if test coverage starts out poor) because by doing so, especially before doing any other changes, you’re making it easier to do some of the other suggestions with confidence. Good read!