That says nothing without context.
If you want to move fast then it's ok to break things, and leaking layers is far from a non-negotiating tradeoff. Software is soft, and you can always revisit a piece of code to refactor it to suit someone's architectural tastes.
It makes no sense to criticize someone for focusing where it matters the most and actually deliver critical features without delay if the tradeoff is some legacy debt, and that by no mean implies they're junior.
One important skill of a senior is being able to keep their eyes on the prize and be smart about tradeoffs. Prioritizing subjective opinions over software architecture over delivering value is more in line with a junior way of thinking about software than not, and to me doesn't sound like a position to criticize others.
The problem is people don't want to expend the very minor extra amount of effort.
So you do it once. Then the next person does it because hey we already do it there so the code is not even subjectively worse, it's objectively the same, cuz you already crossed that line.
After a while you have a giant festering pile of shit that you just keep making bigger and bigger and now people are talking about rewriting it.
In my experience you start actively losing velocity in this situation very quickly. If you're doing this when you still your have 3+ months on a project, you're likely making poor decisions.
I'm not saying you need to constantly gold plate your architecture. But it should be palatable.
The subjective part is what is palatable. But I don't know anyone who thinks steaming turdpiles are palatable.
The interesting thing is that this isn't a technical problem, it's a human problem. Most folks want to clock in, do their job and clock out, and going the extra mile to improve things isn't worth the effort.
I think its the same human tendency that leaves so much pee on the toilet floor - some humans will clean up their own, but almost none will clean up someone else's.
The solution is not to accept "it is what it is".
I suspect this is because it’s almost never rewarded. Either because nobody up the chain of command understands and values quality beyond “it’s working and making us money right now so it’s fine” and also because doing things right is tiring. Why should I spend energy on this and not have that energy when I’m off the clock if I’m not going to get more money/recognition/time off for it and I’ll probably get laid off or move on in a couple years anyway.
I don’t think anyone deliberately starts a job or task thinking they’ll do the absolute minimum. People generally want to do their best but the system moderates this desire and that’s the result.
I don’t really have a solution, just pointing it out.
The time tax is paid by not delivering a feature. Architecture is supposed to be there to help you, not fail deadlines.
Also, it seems you completely failed to read or get what I wrote. My point was that if a PR delivers a feature without defects then your team is far better off if you accept the PR and refactor later when you can spare the time then it is if you throw a hissy fit and refuse to get the feature done if you don't get your way. You are paid to create value which is in delivering features and fixing bugs. Architecture is supposed to be there to help you.
Think about it for a second. If your architectural purity demands a hefty refactor, is that a good justification to postpone a bug fix or delivering a feature? No, it isn't.
> The problem is people don't want to expend the very minor extra amount of effort.
Except that most time it isn't minor.
You have to pass interfaces throughout a bunch of components, which require updating all sorts of tests, and result in a large code footprint.
> After a while you have a giant festering pile of shit (...)
That only happens if you're incompetent at your job and fail to do the most basic maintenance tasks in your code base.
Look at what you're trying to claim. It's supposedly easy and trouble-free to put together a PR that respects your personal notion of what the software architecture should be. Yet, the work to refactor code that breaks it is so insurmountable that you're no longer able to refactor it back to shape?
Which one is it then?
It looks like the FANG engineer you tried to badmouth had a firmer grasp on things and on what it matters the most than you do.
Most deadlines are artificial. If it's not artificial, yeah, you're going to have to sacrifice something for the sake of the real deadline. But make no mistake - you are sacrificing something.
> Also, it seems you completely failed to read or get what I wrote. My point was that if a PR delivers a feature without defects then your team is far better off if you accept the PR and refactor later when you can spare the time
You don't ever have spare time. When you finish a feature there's another waiting for you. At least, that's how it works literally everywhere I've worked. Working this way is just another form of over-promising and under-delivering.
> then it is if you throw a hissy fit and refuse to get the feature done if you don't get your way
Rejecting a change doesn't mean you're throwing a hissy fit.
> You are paid to create value which is in delivering features and fixing bugs. Architecture is supposed to be there to help you.
It does help you. In the medium to longer term. In the short term, literally any standard out there will slow you down - unit tests, CI/CD, shit - testing at all. It all slows you down. It doesn't mean we should just code like its 90s era PHP - the fact that we don't should be a strong signal about the value of this whole hypothesis you have going here.
> You have to pass interfaces throughout a bunch of components, which require updating all sorts of tests, and result in a large code footprint.
Funny. Your tests are slowing you down, why don't you just comment them out to get your feature in?
> Look at what you're trying to claim. It's supposedly easy and trouble-free to put together a PR that respects your personal notion of what the software architecture should be.
It's not actually just personal. We have unit tests that enforce certain architectural rules.
> Yet, the work to refactor code that breaks it is so insurmountable that you're no longer able to refactor it back to shape?
If you don't follow standards, as a senior software engineer, why should anyone else? If no one is following standards, how large do you think that shitpile is going to get? And do you really think its just as easy to fix something after the fact? Of course, if you don't know how something should be, that's one thing. But knowing how it should be and just skipping it altogether is quite another.
Why are you even refactoring for reasons you view as subjective? I don't do subjective refactorings. Despite what you claim, at the point of a change, much of this stuff is very not subjective. If you show any developer a function call before a feature was added, and what it looks like after its been hacked it in some fucked way, no one is going to say the fucked version is better.
> It looks like the FANG engineer you tried to badmouth had a firmer grasp on things and on what it matters the most than you do.
Well maybe you're just so smart its trivial for you to navigate this stuff. If so, continue by all means. Until then, us mere mortals will have to adopt standards so our codebases don't outpace our ability to understand it.
Yes. That’s why it’s an important skill for an engineer to know when to make that tradeoff.
Sounds like you’re agreeing with the grandparent?
I get it, you've been burned out by Enterprise(tm) code. Going to the very edge of the other side isn't any better
I don't. I like to feel productive instead of having to do laundry before getting to the actual work because someone just went ahead without any regard for quality control
>YAGNI + KISS > SOLID
Agreed
Monster architectures are monster architectures, and yet going from Enterprise(tm) architecture to chaos architecture so you can't know anything about the system without carefully going over hundreds of lines of code isn't any better
Also depending on the codebase (and language to some degree), if someone senior writes a lot of overcomplex or abstract code that the rest of the team can't understand/maintain, that's just as much a problem as anything else.
For me this is a much bigger concern than subtle changes to method names. My biggest headache digging into new codebases is when I run into layers and layers of abstraction that save 3 lines of code but force me to construct an entire mental map of the codebase before I can understand how anything works.
Any tips for giving feedback to senior people here? A lot of the complexity from abstraction is very hard to quantify and experienced people can have arguments that sound reasonable.
Like dependency injection is something that a lot of juniors struggle to understand. Or in larger applications DDD and all the crap that goes along with it.
* Interfaces(particularly in java) that have 1 implementation.
* Interfaces with many implementations but where each implementation is used in exactly 1 place.
* large inheritance hierarchies with generic type parameters. I'm sure there's a good use case for this but it's usually a pain.
* Writing "generic" code in an effort to make something re-usable when in reality the code has knowledge of every location it is used in and tightly couples all implementations
With an interface I know that only that method can be called. If a class is passed in i have no idea what methods going to be used. It also gives me a better feeling that the coder has in mind what they need a parameter to do without worrying that they are going to rely on a couple of othogonal methods to be called.
Perhaps a starting point for this discussion is to point out that the DRY principle is nice and all, but there is also WET. Premature abstractions are bad code, a liability and hinder development. It's always better to have two independent but mostly similar codepaths than a strategy pattern with two concrete implementations. If it's cleaner to copy/paste a method and do some tweaks, do that instead. It's not like you can't refactor it when additional use cases emerge, and if they don't emerge then the abstraction wasn't needed to begin with.
Plenty of people believe that being senior is being clever with complex stuff. It isn't. Being senior is to know you don't need complex stuff, and be able to keep as simple as possible. Abstractions go against that.
In my personal experience, the hump from wide-eyed novice junior to productive contributing member really doesn't take that long when every line that everyone writes is code-reviewed. The first couple of code reviews might take longer than usual. But it definitely much less time than it used to take to bring a junior up to speed. No net productivity in the first year used to be a rule, when bringing juniors onboard in the dark ages of computing. With 100% code reviews, that process seems to take no more than a couple of months -- maybe even as little as one month. The difference is dramatic.
It's a fabulous way to transmit not only good culture, but also love of our craft.
I feel this, and have personally experienced submitters taking feedback far too personally, to the point of even using sunken cost as justification of their design, completely overlooking valid points of feedback.
I’m looking for basics. Will it work? is the code clear? is it commented/documented? is the approach simple and maintainable?
Some submitters really need coaching on how to receive feedback, and how to plan their timelines to incorporate revisions.
If I never accept anything from more junior people because it isn’t perfect, how are they ever going to stay motivated?
I aim for one or at most two rounds of review, because anything else is just depressing.
Designing features, architecture and learning should happen before someone even gets to the point of submitting a PR. Plus if it can be improved on in follow PRs then do that instead of blocking juniors with pedantics
Thirty years later, egoless programming is one of Robert Glass's favorite fallacies in 'Facts and Fallacies of Software Engineering.'
IOW, it is a hard problem!
The SE research community has been looking into this for awhile (e.g. Greiler, Bacchelli)
[0] 'The Psychology of Computer Programming' (1971), Gerald Weinberg
[1] 'Facts and Fallacies of Software Engineering', Robert L. Glass
Providing recommendations takes away autonomy from the person who wants to contribute, and it's easy to end up glossing over the real consequences and ending up with "well, I would have done it this way" nit picking reviews.
My job is to ship software, and as an engineer/team lead it is my job to ensure the code we ship is maintainable, tested, and documented as far as any gotchas or hacks go. Ideally we want to improve the code base as well, especially when dealing with legacy code.
I think people take code reviews way too personally. Like, my priority is to ensure I don’t have headaches down the line and when I work with peers or more experience people, I barely leave any comments or get any on my PRs for them since we are all on the same wavelength and for the most part know what we are doing.
A junior engineer is a… junior engineer. They are going to have a lot of comments telling them how to write better code and a good suggestion will include a code suggestion and links to read.
It’s how they will get better, and for bigger tickets or super juniors, I sometimes pair with them to go through their ticket together.
At a certain level you just know how to identify patterns and understand how to build good software.
Juniors tend to just write yolo shit code or add more shit to a legacy shitcode repo instead of trying to improve it. They’ll add to the mess instead of writing a new clean layer on top to which we can easily maintain and modify. But that’s why you have to show them.
The only thing that helps people get better is time, being humble, and being genuinely interested themselves in becoming better at their craft. I remember my junior engineer days, and seeing my PRs filled with comments. But if I didn’t have those I don’t think I’d have leveled up so fast.