10 ms·
My #1 rule for existing codebases: Just because you wouldn't have done it the way they did doesn't mean they did it wrong. I think it's developer nature to loo
by cbanek 8y ago
My #1 rule for existing codebases: Just because you wouldn't have done it the way they did doesn't mean they did it wrong.
I think it's developer nature to look at a huge pile of code that someone else wrote and immediately think: "This is a pile of crap. I can do better, so the first thing to do is rewrite all of this, my way (which just so happens to be _The Right Way_)."
Figure out what you're trying to do, and what is keeping you from doing it. Take an iterative approach to get things done. Realize that after 3 years, they have hopefully fixed a lot of bugs and got to a solution that is somewhat mature and better than you can do in a week.
- amoitnga 8y agoThis. I see myself in your words.
- debt 8y ago"This is a pile of crap. I can do better," I'll take this one step further and say: if you think this, you're unqualified for the position. You are an amateur.
- marcell 8y agoI dunno, I've seen quite a few piles of crap in my day as a SWE. The bar is not that high to do better.
- theossuary 8y agoI have to agree with the comment you're replying to with a caveat. In my (limited) experience balls of mud are created because the developers either didn't understand the problem or its domain well. So if you come into a "pile of crap" and have the same (or less) experience with the problem/domain and (almost definitely) less time, then I really don't think you can do better. Maybe you can do it badly, differently, and think it's better though. If you have more experience in the problem domain (either business or technical), then you might actually be able to rewrite it much better. But then there are quite a few other problems: Do we have time to rewrite it? Will it integrate well? Are you actually solving the right problem? Is it so different people won't know how to use or maintain it? etc. I think even if you're rewriting code, doing it in the framework that's already available, and making as much use of it as is reasonable, is the better option. I don't think it really matters what the solution is, within reason, as long as everyone agrees to follow it.
- fsloth 8y agoThe point is "doing better" is pointless unless it serves a specific business value. Beauty is not a sufficient reason to refactor a code base in production - because refactor always has risks, and those risks should be offset always by some tangible expected reward.
- jamesmus 8y agoDefinitely. "Is it good enough for the business?". If yes, then leave it alone. Try to add new features in a safe way with automated tests. I once worked a contract where it was expressly forbidden by the dev manager to refactor any code unless it was demonstrably necessary to support a new feature. Not much fun for devs at times but I respect the reasons behind the decision (esp. as it was a derivatives trading platfom and it can get pretty expensive pretty quickly when they go wrong).
- maxxxxx 8y agoSometimes it's true but you definitely should wait quite a while before you say it. First you need to understand what the current code does and also learn some history how the code has developed.
- badfrog 8y agoVery many developers have this as their first thought whenever they see somebody else's code for the first time. Likely it's just a response to being confused about how or why something works. It's easier to think that somebody else is bad than to think that you've got something to learn. What matters is how you handle that feeling. If you run with it and continue to operate as though the code is terrible, you're probably not going to get very far as a developer. If instead you take a step back, work to understand the code, and think about what led the original author to make certain decisions, you'll do quite well.
- yakshaving_jgt 8y agoI know this opinion is extremely controversial, but sometimes — usually as a result of inexperience — people do put out bad work. Sometimes it's cheaper for a business to rewrite something (the size of that something is highly context sensitive), than it is to work around a poor approach to a problem. I know we love to believe in the industry that everyone is a genius and any self-doubt is imposter syndrome, but as Camille Fournier put it: "This is Hallmark card pablum".[0] [0]: https://twitter.com/skamille/status/1004735128726376448 https://twitter.com/skamille/status/1004735128726376448
- chillacy 8y agoIf you always think this you’re probably an amateur, but if you seen good code that wasn’t yours, then you might have a sense for things. Of course that requires that you read a lot of code.
- fsloth 8y agoI have no idea who downvoted you. I agree completely. In a business setting all decisions should come from a business analysis, not from an inner desire for beauty. Beauty is important, but save it's search outside business hours.
- james_s_tayler 8y agoI'll take this one step further and say if you truly think trash fires are acceptable and there exists no developer who can replace a trash fire with something warm and inviting then _youre_ the amateur. I get what you mean though. I've seen dreadful results of some people who tried to replace a WinForms app with a WPF app and the people who did the WPF version totally half-baked their underlying framework. It was very amateurish. In contrast the existing framework built on WinForms was 90% baked. It simply had a lifecycle the developer had to understand (very difficult, nobody got it right) and was essentially only missing a final inversion of control layer to tie it all together. I added the missing layer and time to implement a new screen and number of bugs plummeted. I thought it was crazy in 10 years of that code existing and hundreds of people having worked on it that _I_ was the one to spot that and go "hmm, you missed a spot" and have such a dramatic impact with a single week of work. It still fucks with me to this day that _nobody_ was either capable or willing. That just blows my mind. It stuck out like a sore thumb to me. It seems to be the pattern thats emerging in my career though. I've come to understand it's a rare thing. So, fundamentally I agree with you're advice as it applies to most people. I just know first hand there are some people out there that understand exactly how to breathe new life into old systems and how to prevent new systems from decaying.
- emilsedgh 8y agoThis is quite true. But it must be noted that the code may actually be bad. Or it may be bad due to a thousand valid reasons (time pressure, business changes, etc) My personal approach is this: * Write new code must be "good" (Whatever that definition is) * As you iterate through old code, clean them up. For example our own codebase is several years old Nodejs project. Mostly written in callback style with `async.auto` and it's really ugly and hard to maintain comparing to async/await. All our new code uses promises and async/await. All old codebase uses callbacks. We have to use promisify a lot. But without any hiccups, we are slowly and slowly moving towards a better codebase. What helps "immensely" is tests. We have 2 different layers of test and I don't think anything would've been possible without them (although none of them are at 100% coverage. Nowhere near)
- arvinsim 8y agoYes. Trying to refactor everything at one go is a disaster waiting to happen. It is always best to do it incrementally.
- deleted 8y ago[deleted]
- bradleyjg 8y agoI go back and forth if the code I'm adding should be what I consider good or if it's more important that it match the flow and feel of what's already there. A code base written in twelve different ways is usually even harder to read and understand than one that's "not good".
- vonseel 8y agoWell definitely don’t write bad code intentionally!
- teddyh 8y agoCertainly, but the definition of “good” changes over time. At one point, callbacks were good, and now they’re not. What happens when the definition of “good” changes a second time before the code base has been fully converted from callbacks? Well, you shouldn’t write bad code, so now the code has three code styles in it, and, more importantly, two styles of conversions: From callbacks to current-good style and from previous-good style to current good style. This is harder to keep in your head, so conversion now goes more slowly.
- Jach 8y agoThere's a term for this, Chesterton's Fence: https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence > let us say, for the sake of simplicity, a fence or gate erected across a road. The more modern type of reformer goes gaily up to it and says, "I don't see the use of this; let us clear it away." To which the more intelligent type of reformer will do well to answer: "If you don't see the use of it, I certainly won't let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it." That said, if you know how to refactor safely even without tests, you can make improvements. Since the questioner is working in JavaScript, Martin Fowler came out with a second edition of Refactoring with JS code that might be useful. I've never read the first edition (in Java) but Michael Feathers' Working Effectively With Legacy Code is a frequent recommendation of mine. Its main goal is to get your mess under test, ideally tests that help you understand the system as well as improve its quality. It's organized as a set of questions one finds oneself asking in these kinds of code bases and some ways to resolve them: https://www.oreilly.com/library/view/working-effectively-with/0131177052/ https://www.oreilly.com/library/view/working-effectively-wit...
- abraae 8y agoThe counterpoint to Chesterton's Fence is the psychology experiment with the monkeys, the stairs and the banana. This is long but very pertinent to the "that's just how we do things around here" attitude: > This human behavior of not challenging assumptions reminds me of an experiment psychologists performed years ago. They started with a cage containing five monkeys. Inside the cage, they hung a banana on a string with a set of stairs placed under it. Before long, a monkey went to the stairs and started to climb towards the banana. As soon as he started up the stairs, the psychologists sprayed all of the other monkeys with ice cold water. > After a while, another monkey made an attempt to obtain the banana. As soon as his foot touched the stairs, all of the other monkeys were sprayed with ice cold water. It's wasn't long before all of the other monkeys would physically prevent any monkey from climbing the stairs. > Now, the psychologists shut off the cold water, removed one monkey from the cage and replaced it with a new one. The new monkey saw the banana and started to climb the stairs. To his surprise and horror, all of the other monkeys attacked him. After another attempt and attack, he discovered that if he tried to climb the stairs, he would be assaulted. > Next they removed another of the original five monkeys and replaced it with a new one. The newcomer went to the stairs and was attacked. The previous newcomer took part in the punishment with enthusiasm! Likewise, they replaced a third original monkey with a new one, then a fourth, then the fifth. Every time the newest monkey tried to climb the stairs, he was attacked. The monkeys had no idea why they were not permitted to climb the stairs or why they were beating any monkey that tried. > After replacing all the original monkeys, none of the remaining monkeys had ever been sprayed with cold water. Nevertheless, no monkey ever again approached the stairs to try for the banana. Why not? Because as far as they know that's the way it's always been around here.
- userbinator 8y ago"When you see something that's old and it's been there for a long time and it's working, don't laugh at it." (From a not-really-related yet quite interesting talk I watched a while back: https://www.youtube.com/watch?v=TQB0KK2rxcw#t=31m0s https://www.youtube.com/watch?v=TQB0KK2rxcw#t=31m0s --- also featured on HN at https://news.ycombinator.com/item?id=18430512 https://news.ycombinator.com/item?id=18430512)
- ehnto 8y agoI think it's beneficial to work within the idioms of an existing codebase to maintain predictability and reduce complexity overall. If you can improve the code by removing complexity, that's a win. Otherwise idiomatic code will stand the test of time in a complex codebase even if it isn't how you would have written it. One bad way of doing something is better than two completely different ways of doing the same thing.
- james_s_tayler 8y agoMy number #1 rule for existing codebases: Get it into a state that will support current business goals and roadmap while protecting current business value and users. That definitely can and often should include refactoring. But. There are many caveats and your rule is absolutely one we should heed. Though I would hate it for the capable, talented, motivated individuals that are part of every informal (or formal) clean up crew to heed it at the wrong times. The times when (a) that's not the way anyone should do it (b) it's actively adding negative value along some dimension and (c) it's preventing the business from reaching the next level. I think it's a skill in its own right to identify solutions that do and don't need replacing. I think a good rule of thumb is to ask "will this enable some new capability that we haven't been able to do before and that would deliver real value along some dimension to more than just me?". If you don't have a solid yes to that you shouldn't do it. Often I find if you can get an order of magnitude better along one or multiple (preferably multiple) dimensions then it's usually worth the effort.
- crimsonalucard 8y agoSometimes the initial design truly was a mistake.
- cc81 8y agoYes, and sometimes the people are fully aware of that as well. But even a shitty designed piece of code that has been running in production for year will have ironed out most of its bugs. There might be other designs that would be more maintainable but refactoring to reach that might be an investment that the company is not willing to make.
- Illniyar 8y agoI don't think that's the right mindset to approach it. My experience usually is that it is a pile of crap, and I can do better, but I probably would make the dame pile of crap if I was under the same conditions of the original coders (time constraints and scope creep usually) So change it if you can, but respect those who were there before
- rjf72 8y agoI'd also add here that hindsight is 20/20. You don't really know what you're creating until it's created. Do it again a second time when you know exactly what you're getting at the end, alongside the challenges you'll face in the process of such, and you'd be able to do it faster, cleaner, and just overall better.
- cbanek 8y agoI agree with both you and the parent that sometimes rewriting the code is best, and you can usually make a superior solution by utilizing lessons learned from the first version, either from rewriting or iterative improvement. But I think the mindset to approach a codebase with is to be open to it, rather than dismissive of it. Code is such a stylized thing, and everyone has their own style and loves their own style most of all. It's hard and takes time to understand someone else's code (a vastly underrated skill IMHO). The temptation to find some reason to start over when it isn't exactly necessary is usually quite high. Once you understand what the current code is doing, then you really have to apply your good skills on what parts to save, and what to change, and how that relates to the schedule and cost. Also in a corporate setting, there can be political / organizational friction to throwing out code or starting over (and it rarely makes you friends with whoever wrote the first version).
- gorbachev 8y agoExactly! The first version of anything almost certainly ALWAYS sucks. Usually the only way that doesn't happen is if the team developing the solution has prior knowledge of some kind...having built a similar solution before. I used to always hate consulting for this reason. The company I was working for was always doing greenfield development for clients. We never, ever got to do v2, because the projects always transitioned to clients' teams. I can't remember a single project I was working on that I was happy with.
- rhizome31 8y agoDead code, inconsistent naming, CPOLD versioning, god objects, giant functions with christmas-tree like block nesting, SRP violations everywhere... A lot of code written by professional developers is like this and this is objectively bad. I've been lucky enough to work on several occasions with code that wasn't like this and it was a joy to maintain. Cherry on the cake, that type of code usually comes with some automated tests.
- james_s_tayler 8y agoDon't forget the customary "Utils" class which always contains lots of hidden gems like reimplementing 75% of the string manipulation functions provided by the standard library - some being unique implementations, others being a single call to the underlying standard library :)
- ummonk 8y agoI mean it's pretty normal for a long existing codebase to be have had a lot of poor (in hindsight) design / architecture decisions in it and feel that if you got to write it with the benefit of hindsight you'd do a better job. It's only hubris to think you can do it in a week, rather than requiring as much time as the original (but with a better resulting codebase).
- cynicalreason 8y agoI'm doing exactly this now ... the problem is multiple iterations of trying to set an architecture and a pattern to where things go. The architecture presented, if it was to be implemented that way would have been perfectly fine. Now there are 3 iterations .. with 3 different patterns of what would be a state container, UI business flows split across 3 layers (data access/api, business/decision , view models) and view models (NOT VIEWS) sharing state. What also doesn't help is the tooling is close to non existent .. I got so used to webpack / lib(framework) dev tools / hot reloading / all in 1 dependency management / intellisense
- rdiddly 8y agoIf your predecessors were indeed total hacks at programming, you can get all of the above. But it's a dubious pleasure. My head sustained a lot of scratching and smacking and my face was over-palmed.
- speedplane 8y agoSometimes a very dubious pleasure. I work at a company where another product team decided to rewrite the entire product from Angular 1 to Angular 2 because it was so much better. I was shocked and dismayed, but didn't say much because I didn't feel like starting a fight I would lose, and it would suck me into a project that wasn't my primary focus.
- james_s_tayler 8y agoIt's not that they were total hacks. It's that they fell into the 1 standard deviation of the normal distribution that made them an average developer. Average devs produce the status quo, which is "software that works but that everyone complains about having to maintain". In my experience people with solid framework design and system architecture skillsets invariably fall into the top tail end of the distribution. They aren't like other developers. And they're rare. They can and often do get dubious pleasure out of making old code new again.
- ken 8y agoIt's developer nature to look at any code and think that -- especially your own! Or if you prefer the abstraction, perhaps to consider that "myself 3 months ago" is a different person than "myself today". Every time I've gazed upon a new legacy codebase and said "This looks like junk", the system architect responded "Yes -- that's why you're here!"