5 ms·

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. Ever

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).
jacekm 7y ago
I think respect does not imply assumption of correctness. It's more like: there's a reason why the code looks like it does. The reason could be time pressure, some crucial information missing at the time when it was being implemented, etc. By all means try to improve it but don't look down on a person who created it. It's easy to judge from perspective of 6 or more months.
- mgkimsal 7y ago
  "The reason" can also be lack of basic competence in previous workers. That can be extended to the rest of the team or management around the worker(s) for not providing a guidance/help. "reason" can be "cause/effect" reason, but also "justifiable rationale". The files look like they do because someone typed keys and hit 'save' - cause/effect. But there's often no justification for how some code exists in its state that has to do with issues related to the business/logic itself. External factors - time (as you mentioned) or missing info (as you mentioned) - that missing info should have been documented somewhere. I put out a lot of bad code in my early days. Seniors who came in afterwards ... yeah, the only or primary reason they could get from my code was "he wasn't very good at this". And they were correct. I got better, but primarily through trial and error (lots of them). In thinking of some specific projects from 98/99 - I was demonstrably not good. I got stuff done, but it was generally very inefficient. But... it worked (to the level of understanding of everyone on the project). Even then, people weren't coming in after me to make basic stuff work, but they did help make it better. If you're coming in to a project and there's no unit tests, no sample data, no repeatable build process, no documentation, no testing or verification process, and no previous team members to actually talk to to get questions answered, there's little reason to have any assumption of correctness. There may very well be 'reasons' for the lack of all of those factors above, but nothing makes me assume the code is correct. And... if I'm brought in to a project like that, "respecting" the code or the people before adds no benefit to the project, and may detract from getting stuff done.
  - vorpalhex 7y ago
    It's not an assumption of correctness. It's an assumption of purpose. Chesterton's fence[1]. Let's say you inherit a large codebase, and it in fact does not have any tests, and is relatively complex and convoluted. Obviously that's not going to be very maintainable without testing, and it may make sense to do a rewrite... But first, write tests. Test your assumptions. Read the commit history and see how the code base evolved over time. Maybe the reasons it was built the way it was are dumb ("I like naming my variables after my favorite flavors of pie!") or maybe it was done for really good reasons ("The database library breaks with odd numbers of connections, so the pool always has to be an even number."). The issue is that you simply don't know and risk repeating your ancestors mistakes unless you do some investigation. [1] - https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence
mrfredward 7y ago
It's very situation dependent. I try to advocate for the old code when I can, but I've had similar experiences. The most recent example was a multithreaded app started by a person who understood neither mutexes nor pure functions. Doing a ctrl+f for "sleep" for quickly showed that code was doomed to always be breaking in inexplicable ways. Give code the benefit of the doubt as much as possible, but there is a point where old code deserves more suspicion than respect.
teddyuk 7y ago
"respect the existing code" <-- for me, this means respect the people who wrote it especially if they are still about. Don't just trash it, learn it, understand it, talk about why it could be improved - people write crap code for many different reasons. Do replace it but do it with valid reasons and without being horrible.
- mgkimsal 7y ago
  if they're still around, you can ask questions, and get a much better handle on why it's the way it is, and perhaps avoid earlier mistakes, and come up with a better strategy. maybe even just actually document the 'whys' and leave it as is.
candu 7y ago
Fair point: sometimes things are, in fact, bad enough that a rewrite is warranted. IMHO, this is where the "senior engineer" mantle becomes less about engineering and more about interpersonal / organization-navigational skills. Let's say I'm a manager at said company. An engineer who just joined my project - which our previous senior engineer, who I trust (fairly or not), built - tells me it needs a complete rewrite. My default reaction is likely skepticism: the received wisdom is that rewrites are seldom justified; your assertion contradicts both this wisdom and my previous senior engineer. What do you expect my answer will be? As the engineer making this recommendation, you'll need to do a bit of showing why. Part of this is technical: write some tests that show the problem, quantify the data loss, etc. Show that you've carefully balanced rewrite vs. other more incremental approaches to fixing the problem. Prototype part of a newer system quickly and show that it can be done. The harder part to motivate - and often the more important part when it comes to "how do I get people to say yes?" - is non-technical. What's the business impact? What's the user impact? Is this an existential risk to the company if not fixed? How so? How do we assure our clients / users / etc. that time spent here is more important than time spent on features they want? Can you get someone with significant decision-making power to agree? How will you de-risk the rewrite and eventual migration, neither of which are trivial? How can you get just enough buy-in to secure the time needed to answer these questions, show quick progress, and reassure others that this won't be an even bigger catastrophe?
arendtio 7y ago
I think you are missing the point. Respecting systems doesn't mean 'not touching' or 'working around' existing systems. It just means 'understand it, before you change/rewrite it'. Otherwise, your rewrite might be even worse. I have seen people proclaiming a rewrite with a newer technology would be a good idea who didn't have the slightest clue about the system they were trying to replace (except that it was build with some outdated technology) and I am 85% sure, that their rewrite would have been worse. When I confronted them with my claims, they gave up quickly. As a Senior Engineer, you should know what you are doing. If you see obviously bad code, thinking about a rewrite is no sin. Just try to understand what caused the current system to look like it does.

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).

jacekm 7y ago

I think respect does not imply assumption of correctness. It's more like: there's a reason why the code looks like it does. The reason could be time pressure, some crucial information missing at the time when it was being implemented, etc. By all means try to improve it but don't look down on a person who created it. It's easy to judge from perspective of 6 or more months.

mgkimsal 7y ago
"The reason" can also be lack of basic competence in previous workers. That can be extended to the rest of the team or management around the worker(s) for not providing a guidance/help. "reason" can be "cause/effect" reason, but also "justifiable rationale". The files look like they do because someone typed keys and hit 'save' - cause/effect. But there's often no justification for how some code exists in its state that has to do with issues related to the business/logic itself. External factors - time (as you mentioned) or missing info (as you mentioned) - that missing info should have been documented somewhere. I put out a lot of bad code in my early days. Seniors who came in afterwards ... yeah, the only or primary reason they could get from my code was "he wasn't very good at this". And they were correct. I got better, but primarily through trial and error (lots of them). In thinking of some specific projects from 98/99 - I was demonstrably not good. I got stuff done, but it was generally very inefficient. But... it worked (to the level of understanding of everyone on the project). Even then, people weren't coming in after me to make basic stuff work, but they did help make it better. If you're coming in to a project and there's no unit tests, no sample data, no repeatable build process, no documentation, no testing or verification process, and no previous team members to actually talk to to get questions answered, there's little reason to have any assumption of correctness. There may very well be 'reasons' for the lack of all of those factors above, but nothing makes me assume the code is correct. And... if I'm brought in to a project like that, "respecting" the code or the people before adds no benefit to the project, and may detract from getting stuff done.
- vorpalhex 7y ago
  It's not an assumption of correctness. It's an assumption of purpose. Chesterton's fence[1]. Let's say you inherit a large codebase, and it in fact does not have any tests, and is relatively complex and convoluted. Obviously that's not going to be very maintainable without testing, and it may make sense to do a rewrite... But first, write tests. Test your assumptions. Read the commit history and see how the code base evolved over time. Maybe the reasons it was built the way it was are dumb ("I like naming my variables after my favorite flavors of pie!") or maybe it was done for really good reasons ("The database library breaks with odd numbers of connections, so the pool always has to be an even number."). The issue is that you simply don't know and risk repeating your ancestors mistakes unless you do some investigation. [1] - https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence

mrfredward 7y ago

It's very situation dependent. I try to advocate for the old code when I can, but I've had similar experiences. The most recent example was a multithreaded app started by a person who understood neither mutexes nor pure functions. Doing a ctrl+f for "sleep" for quickly showed that code was doomed to always be breaking in inexplicable ways. Give code the benefit of the doubt as much as possible, but there is a point where old code deserves more suspicion than respect.

teddyuk 7y ago

"respect the existing code" <-- for me, this means respect the people who wrote it especially if they are still about. Don't just trash it, learn it, understand it, talk about why it could be improved - people write crap code for many different reasons. Do replace it but do it with valid reasons and without being horrible.

mgkimsal 7y ago
if they're still around, you can ask questions, and get a much better handle on why it's the way it is, and perhaps avoid earlier mistakes, and come up with a better strategy. maybe even just actually document the 'whys' and leave it as is.

candu 7y ago

Fair point: sometimes things are, in fact, bad enough that a rewrite is warranted. IMHO, this is where the "senior engineer" mantle becomes less about engineering and more about interpersonal / organization-navigational skills. Let's say I'm a manager at said company. An engineer who just joined my project - which our previous senior engineer, who I trust (fairly or not), built - tells me it needs a complete rewrite. My default reaction is likely skepticism: the received wisdom is that rewrites are seldom justified; your assertion contradicts both this wisdom and my previous senior engineer. What do you expect my answer will be? As the engineer making this recommendation, you'll need to do a bit of showing why. Part of this is technical: write some tests that show the problem, quantify the data loss, etc. Show that you've carefully balanced rewrite vs. other more incremental approaches to fixing the problem. Prototype part of a newer system quickly and show that it can be done. The harder part to motivate - and often the more important part when it comes to "how do I get people to say yes?" - is non-technical. What's the business impact? What's the user impact? Is this an existential risk to the company if not fixed? How so? How do we assure our clients / users / etc. that time spent here is more important than time spent on features they want? Can you get someone with significant decision-making power to agree? How will you de-risk the rewrite and eventual migration, neither of which are trivial? How can you get just enough buy-in to secure the time needed to answer these questions, show quick progress, and reassure others that this won't be an even bigger catastrophe?

arendtio 7y ago

I think you are missing the point. Respecting systems doesn't mean 'not touching' or 'working around' existing systems. It just means 'understand it, before you change/rewrite it'. Otherwise, your rewrite might be even worse. I have seen people proclaiming a rewrite with a newer technology would be a good idea who didn't have the slightest clue about the system they were trying to replace (except that it was build with some outdated technology) and I am 85% sure, that their rewrite would have been worse. When I confronted them with my claims, they gave up quickly. As a Senior Engineer, you should know what you are doing. If you see obviously bad code, thinking about a rewrite is no sin. Just try to understand what caused the current system to look like it does.