6 ms·

About this one: > Respect code and systems that came before you. There are reasons for every code and every guard that exists in production Sometimes there's

About this one: > Respect code and systems that came before you. There are reasons for every code and every guard that exists in production Sometimes there's bad code in prod, or code that doesn't need to exist anymore. You should try to understand when something is there for a good reason versus a bad reason. Cut out the bad code, keep the good code. I've seen a lot of cases where people assume that current code is untouchable or designed very intentionally, so they wrap it and then you end up with a tower of abstractions built on a poor base. So, I think as a senior engineer you should try to understand the difference between code that should stay and code that should go.

manojlds 7y ago
I think the main thrust is respect the code and understand why it was there. Don't start with the possible initial feeling that a particular piece of code or guard is not needed. If you do find that it's not needed, remove it or fix it.
- Pawka 7y ago
  I'll second manojlds's comment. If there is bad code/design decision on production - it not necessary means it was bad all the time. Software grows, requirements changes and it is very likely there is a reason why the code is as it is now. Even if it does not solve the problem efficiently today. People also tend to forget human and business impact. The code might look different if there is 1 week or 4 weeks given for implementation. Timelines, pressure from management/stakeholders to deliver faster has impact to made solutions and implementations. That is when respect is needed - having empathy to understand why this part of software looks like it is now.
- IanPBann 7y ago
  This is very similar to Joel Spolsky's advice that you should never just chuck code out and and start from scratch. https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/ https://www.joelonsoftware.com/2000/04/06/things-you-should-...
  - littleblah 7y ago
    Yup. That was my inspiration, and sadly, I have been on the side as well where things started from scratch, and ended up blowing up couple of months later.
- icebraining 7y ago
  It's essentially the Chesterton's Fence principle.
mgkimsal 7y ago
The opposite can happen. Took over a system in Aug '18 with someone else. Previous 'senior engineer' had left after a year to go be a CTO someplace else. Everything seemed wrong - bad smells everywhere, spidey-sense a-tinging every day. In October, we found that code put in place had been losing data since April - we had 6 months of pure data loss. Data that people assumed was 'working' because ... they hit a button and looked at the screen, and it was there. HOWEVER... after the midnight job run, the data was gone. No one ever looked the next day. Until October, and then it was way too late. The whole "respect the existing code" has a HUGE assumption in it - that things are actually WORKING CORRECTLY in the first place. It also often assumes some of the original people who wrote the code you're dealing with are still on the team and and to answer questions. THAT particular project above - my gut was "rebuild" - and we didn't. 16 months later, we are still finding code that is simply broken - data is lost, things are breaking, etc. But the whole "well, you're just someone coming in new - all devs just want to start from scratch and you shouldn't do that!" reared its head, and that idea was pushed aside. I'd turn it around. Someone needs to be able to demonstrate - usually via automated tests I'd think - that things are, in fact, working as expected/assumed. If not, study/understand, then feel free to rebuild/throwaway. I've taken part in another project with hundreds/thousands of tests, and while I hit code that looks/smells weird sometimes, some of the original people are on the team still, answer questions about 'why', and the tests pass. Even though it sometimes looks 'bad', it's functional and we can demonstrate its functional and working. This is a HUGE difference, and that assumption about "it's working correctly right now" shouldn't be taken for granted.
- DarkWiiPlayer 7y ago
  That's why I think code that seems "weird" should always be accompanied by a comment explaining why it is the way it is and why a seemingly more obvious approach wasn't taken.
  - mgkimsal 7y ago
    At the very least. Better might be a comment linking to a ticket or wiki page or internal README with far more explanation. What's the technical reasoning, and what was the business impetus for requiring this done this way at this time? Again, I've been on both sides of this fence, but the "never rebuild" mantra has a lot of assumptions built in, chief being that things are actually working correctly. If they are, and there's no tests around it, memorialize the existing state with your own tests at the very start. It's what we did on the August project referenced above, and it took weeks (months, really) to have a modicum of basic tests ensuring some base level of understanding for our team (just 2 of us).
msluyter 7y ago
AKA, Chesterson's Fence: https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence
KaiserPro 7y ago
respect doesn't mean revere. Replacing something simply because its not code you authorised or written isn't respectful. Replacing something because its not on a shiny platform, took too long to figure out, or simila, again isn't respectful. replacing broken code, fixing bugs, changing architecture to adapt to another situation is fine.
dahart 7y ago
> you should try to understand the difference between code that should stay and code that should go. I think that’s exactly what the rule is saying, I think almost everyone would agree. The problem is assuming that code is bad, or even actively looking for “bad”. If it’s in prod, that usually means it worked and was needed. Engineers in my experience do like to assume things are bad and actively look for reasons to re-engineer things without properly accounting for the number of ways things are going right. During my career, I’ve watched two separate large teams at two separate companies decide to rewrite a major codebase because they thought there was too much bad, and they thought there was a tower of abstraction. The result was they both dramatically underestimated the time needed to rewrite, and after having wasted literally tens of millions of dollars, ended up with roughly the same code quality as before and a couple of years of unnecessary down time. It’s more difficult to understand the code that came before and the reasons why it’s there, and it’s easier to judge it bad and decide to rewrite it without completely understanding it, that’s why the default should be to assume the code is there for a good reason, and try harder to understand the reasons. That rule should, of course, be balanced with this rule, both are true: > Simplify code, systems, and architectures relentlessly I personally suspect that writing down and maintaining all the requirements and lifetimes for the codebase separately from the codebase, and having a regression test suite with complete coverage, is the way to safely deprecate features and continue to simply old large complicated codebases. But I’ve never seen that work in practice, yet...
- dbmikus 7y ago
  I don't think the OP meant "don't touch old code", but this sentence, to me, implies that "reasons" means good reasons: > There are reasons for every code and every guard that exists in production. Sometimes the reasons are not good ones. Maybe that's exactly what the author meant. The rule doesn't read like that to me, though. And yeah, if you're rewriting something, don't rewrite the whole damn thing! Good code is modular and you should be able to pick it apart and replace components that are causing problems. If it's not causing a problem, don't rewrite it... However if you're going to be adding features to a section of code (or otherwise working on it a lot), it's a good time to try to understand if you can do a bit of refactoring along the way to clean it up. By default, all code gains code smell over time and also becomes more robust. It's a tight rope to walk.

manojlds 7y ago

I think the main thrust is respect the code and understand why it was there. Don't start with the possible initial feeling that a particular piece of code or guard is not needed. If you do find that it's not needed, remove it or fix it.

Pawka 7y ago
I'll second manojlds's comment. If there is bad code/design decision on production - it not necessary means it was bad all the time. Software grows, requirements changes and it is very likely there is a reason why the code is as it is now. Even if it does not solve the problem efficiently today. People also tend to forget human and business impact. The code might look different if there is 1 week or 4 weeks given for implementation. Timelines, pressure from management/stakeholders to deliver faster has impact to made solutions and implementations. That is when respect is needed - having empathy to understand why this part of software looks like it is now.
IanPBann 7y ago
This is very similar to Joel Spolsky's advice that you should never just chuck code out and and start from scratch. https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/ https://www.joelonsoftware.com/2000/04/06/things-you-should-...
- littleblah 7y ago
  Yup. That was my inspiration, and sadly, I have been on the side as well where things started from scratch, and ended up blowing up couple of months later.
icebraining 7y ago
It's essentially the Chesterton's Fence principle.

mgkimsal 7y ago

The opposite can happen. Took over a system in Aug '18 with someone else. Previous 'senior engineer' had left after a year to go be a CTO someplace else. Everything seemed wrong - bad smells everywhere, spidey-sense a-tinging every day. In October, we found that code put in place had been losing data since April - we had 6 months of pure data loss. Data that people assumed was 'working' because ... they hit a button and looked at the screen, and it was there. HOWEVER... after the midnight job run, the data was gone. No one ever looked the next day. Until October, and then it was way too late. The whole "respect the existing code" has a HUGE assumption in it - that things are actually WORKING CORRECTLY in the first place. It also often assumes some of the original people who wrote the code you're dealing with are still on the team and and to answer questions. THAT particular project above - my gut was "rebuild" - and we didn't. 16 months later, we are still finding code that is simply broken - data is lost, things are breaking, etc. But the whole "well, you're just someone coming in new - all devs just want to start from scratch and you shouldn't do that!" reared its head, and that idea was pushed aside. I'd turn it around. Someone needs to be able to demonstrate - usually via automated tests I'd think - that things are, in fact, working as expected/assumed. If not, study/understand, then feel free to rebuild/throwaway. I've taken part in another project with hundreds/thousands of tests, and while I hit code that looks/smells weird sometimes, some of the original people are on the team still, answer questions about 'why', and the tests pass. Even though it sometimes looks 'bad', it's functional and we can demonstrate its functional and working. This is a HUGE difference, and that assumption about "it's working correctly right now" shouldn't be taken for granted.

DarkWiiPlayer 7y ago
That's why I think code that seems "weird" should always be accompanied by a comment explaining why it is the way it is and why a seemingly more obvious approach wasn't taken.
- mgkimsal 7y ago
  At the very least. Better might be a comment linking to a ticket or wiki page or internal README with far more explanation. What's the technical reasoning, and what was the business impetus for requiring this done this way at this time? Again, I've been on both sides of this fence, but the "never rebuild" mantra has a lot of assumptions built in, chief being that things are actually working correctly. If they are, and there's no tests around it, memorialize the existing state with your own tests at the very start. It's what we did on the August project referenced above, and it took weeks (months, really) to have a modicum of basic tests ensuring some base level of understanding for our team (just 2 of us).

msluyter 7y ago

AKA, Chesterson's Fence: https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence

KaiserPro 7y ago

respect doesn't mean revere. Replacing something simply because its not code you authorised or written isn't respectful. Replacing something because its not on a shiny platform, took too long to figure out, or simila, again isn't respectful. replacing broken code, fixing bugs, changing architecture to adapt to another situation is fine.

dahart 7y ago

> you should try to understand the difference between code that should stay and code that should go. I think that’s exactly what the rule is saying, I think almost everyone would agree. The problem is assuming that code is bad, or even actively looking for “bad”. If it’s in prod, that usually means it worked and was needed. Engineers in my experience do like to assume things are bad and actively look for reasons to re-engineer things without properly accounting for the number of ways things are going right. During my career, I’ve watched two separate large teams at two separate companies decide to rewrite a major codebase because they thought there was too much bad, and they thought there was a tower of abstraction. The result was they both dramatically underestimated the time needed to rewrite, and after having wasted literally tens of millions of dollars, ended up with roughly the same code quality as before and a couple of years of unnecessary down time. It’s more difficult to understand the code that came before and the reasons why it’s there, and it’s easier to judge it bad and decide to rewrite it without completely understanding it, that’s why the default should be to assume the code is there for a good reason, and try harder to understand the reasons. That rule should, of course, be balanced with this rule, both are true: > Simplify code, systems, and architectures relentlessly I personally suspect that writing down and maintaining all the requirements and lifetimes for the codebase separately from the codebase, and having a regression test suite with complete coverage, is the way to safely deprecate features and continue to simply old large complicated codebases. But I’ve never seen that work in practice, yet...

dbmikus 7y ago
I don't think the OP meant "don't touch old code", but this sentence, to me, implies that "reasons" means good reasons: > There are reasons for every code and every guard that exists in production. Sometimes the reasons are not good ones. Maybe that's exactly what the author meant. The rule doesn't read like that to me, though. And yeah, if you're rewriting something, don't rewrite the whole damn thing! Good code is modular and you should be able to pick it apart and replace components that are causing problems. If it's not causing a problem, don't rewrite it... However if you're going to be adding features to a section of code (or otherwise working on it a lot), it's a good time to try to understand if you can do a bit of refactoring along the way to clean it up. By default, all code gains code smell over time and also becomes more robust. It's a tight rope to walk.