Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Show HN: A Senior Engineer's CheckList

news.ycombinator.com

Post reply on HN
Print view View original

131–140 of 155 posts

Re: Show HN: A Senior Engineer's CheckList

#131

Post by candu »

mgkimsal wrote: post #96
dbmikus wrote: post #53

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…

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…

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?

Re: Show HN: A Senior Engineer's CheckList

#132

Post by roland35 »

jcadam wrote: post #93
dagw wrote: post #74

Earlier quoted context omitted.

I've been a "senior" engineer for years, in that time I've read through a few dozen CVs, sat in on 2 interviews, and had final say in hiring of exactly zero people. I suspect the same is true for at least half the senior people here, unless they also happen to managers.

I remember myself and every other senior engineer on a team once rejecting a candidate and being overruled by management. The guy was hired and... turned out to be as useless as we thought he'd be.

This happened at a company I worked at as well - except the rejected candidate became our manager!

Re: Show HN: A Senior Engineer's CheckList

#134

Post by mgkimsal »

teddyuk wrote: post #127
mgkimsal wrote: post #96

Earlier quoted context omitted.

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…

"respect the existing code" 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.

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.

Re: Show HN: A Senior Engineer's CheckList

#135

Post by geodel »

mgkimsal wrote: post #125
vorpalhex wrote: post #110

Earlier quoted context omitted.

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…

> Read the commit history You're assuming there's always 'commit history' to review :) I completely agree you need to do investigation, and document what you can about the system. Some of that documentation will take the form of tests. Without a doubt. The 'assumption' I refer to is either clients or other devs assuming something is correct. I've lost track of the number of times I've heard "it was working fine until…

I think you are making some great points here. I have seen code commits are mostly right click in Eclipse and commit all modified files irrespective of if they are local config changes or preferences.

The senior devs in projects I worked have made 10 level deep 'strategy patterns' for future enhancements which still had hardcoded values underneath and absolutely non-extensible.

So in my books senior developers knew what they were doing comes with huge assumption. I have noticed many sr engineers simply had delusions of grandeur far beyond their programing skills.

Re: Show HN: A Senior Engineer's CheckList

#136

Post by dbmikus »

dahart wrote: post #116
dbmikus wrote: post #53

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…

> 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 r…

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.

Re: Show HN: A Senior Engineer's CheckList

#138

Post by dahart »

dbmikus wrote: post #136
dahart wrote: post #116

Earlier quoted context omitted.

> 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 r…

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…

It is true, sometimes the reasons are not good ones. But, if it is in production, one should assume the reasons are good ones, there's already some evidence. The point is mainly that the burden of proof is on the reader to demonstrate the reasons are not good before modifying it... especially because you're right: code smell is the default product of time. We have to be careful about assuming that code smell indicates something is wrong. My couple of decades of experience is that people assuming reasons aren't good and jumping into modifications is a bigger acute problem than being more conservative than necessary. Accumulating the tower of abstractions is no doubt a problem, but it happens more slowly and is caused by a multitude of bad practices, it rarely breaks production or causes downtime, unlike refactoring too eagerly because of smell or opinion rather than demonstrated necessity. It is indeed a fine line, I completely agree!

Re: Show HN: A Senior Engineer's CheckList

#139

Post by gfodor »

5trokerac3 wrote: post #111
mgkimsal wrote: post #105

Earlier quoted context omitted.

"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 th…

> "The reason" can also be lack of basic competence in previous workers. That type of wording belies a destructive approach to the problem. What I'm told is "lack of competence" is often someone not being trained properly, or leadership that decided to make a deadline no matter what, or a junior dev that got called a senior dev to fill a seat at a cheaper rate, or someone who knew they didn't have the skills but didn…

Often, but not always. When people generalize, it's an equal error to conjecture that the premise for the generalization is always false in the other direction. Lack of skill/experience is just a nicer way of saying incompetence, and sometimes this is a proximate cause, and sometimes this is actually a root cause, in cases where the person was a genuine bad actor who fooled others into thinking they were qualified for the position.

Now of course there are many times where this is used as a crutch to blame system problems, but I've found a trend towards the idea that it's never due to lack of skill/experience on the part of an individual, and always due to some other root cause. That seems equally incorrect, and good judgement about the cause should be agnostic to how one feels about the idea of saying an individual failed at their jobs (which usually isn't a pleasant feeling for most.)

On the upside, one should actually expect people to fail at their jobs occasionally within certain boundaries, in order for them to grow. So failures to due to 'incompetence' (not negligence) should be often accepted and blameless, given they should be happening occasionally if you are pushing people past their limits. The best way to ensure someone doesn't make large categories of mistakes is to let them make one with a small blast radius, and having them take responsibility for addressing the failure and remediations. (This isn't at odds with blamelessness -- most ethical people will accept responsibility for their failures and correcting them, if they are able to fail in an open, respectful environment)

Re: Show HN: A Senior Engineer's CheckList

#140

Post by mgkimsal »

geodel wrote: post #135
mgkimsal wrote: post #125

Earlier quoted context omitted.

> Read the commit history You're assuming there's always 'commit history' to review :) I completely agree you need to do investigation, and document what you can about the system. Some of that documentation will take the form of tests. Without a doubt. The 'assumption' I refer to is either clients or other devs assuming something is correct. I've lost track of the number of times I've heard "it was working fine until…

I think you are making some great points here. I have seen code commits are mostly right click in Eclipse and commit all modified files irrespective of if they are local config changes or preferences. The senior devs in projects I worked have made 10 level deep 'strategy patterns' for future enhancements which still had hardcoded values underneath and absolutely non-extensible. So in my books senior developers knew w…

Exactly.

> So in my books senior developers knew what they were doing comes with huge assumption.

This is another big assumption, and... especially given that we know in our industry that 'title inflation' is a real thing... "sr" just doesn't seem to mean much. I'm seeing lots of job postings today that call for "sr foo engineer", calling for 3+ years of experience. My definition and expectations of 'sr' are far different.

Interestingly, I've only been in a couple of places that even defined what they actually meant by that title - what was expected was written down. It was still open to interpretation, but a baseline to judge you against, and to give jr folks something to shoot for.

> which still had hardcoded values underneath and absolutely non-extensible.

Don't even get me started on people that just learned the 'final' keyword and abuse the hell out of it. :)

Post reply on HN