In my experience no coding standards or cleanliness can prevent this. The only sure way is testing on commit/PR.
EDIT: A common failure mode I've noticed is that people only test the functionality they've added and not other parts of the system that may be affected.
I think that's an issue with how people define "clean." It isn't just about surface aesthetics. Think of a cooking appliance that looks "clean" in its design, that might win a designy design award, but accumulates gunk in hard-to-reach crevices because it was only designed to look clean, not designed to be kept actually clean.
What makes it easy to make changes, add features, find bugs, etc. is good design.
Testing can help find problems, sure, but you can't test in quality (you can only, to a degree, test in robustness).
The difference between a decent design and a mess is mostly what happens when your "simple, local fix" causes unexpected tests to fail. If your first reaction is "damn, guess we can't do that" three is a good chance you have a mess on your hands.
Clean code (yes some of this is just taste) is mostly a symptom that people have been thinking about this stuff, not a guarantee.
There is a corollary though, which is difficult. Quality of design and techniques used mostly put an upper bound on the complexity the system can have before it becomes unmanageable. If it grows long enough though, it will get there.
This failure mode often comes from parts of the system being coupled when they shouldn't be, or it's not obvious that they are.
Have you ever had a change to a logging statement break cause some other code to stop functioning? I have. And I was thoroughly disappointed by the person who made a logging statement have any effect outside of logging.
Clean code needs to adhere to principles such as the Principle of Least Astonishment[0], otherwise code is bound to break when other people modify it.
[0] https://en.wikipedia.org/wiki/Principle_of_least_astonishmen...
A nicer way to say it (from pair programming ethos): If someone asks you a question to clarify some code, then it needs a comment (or a re-write/re-factor). Note that I didn't say anything about the experience level of the asker.
One more thing: I've met (multiple) people who are firmly in the "Convention over configuration" camp and the "Principle of Least Astonishment" camp and cannot understand how new joiners are continuously surprised by this or that convention.
At $different_employ, there was lots of thrashing of teeth pertaining to `yarn audit` running against the codebase, and failing people's builds. Some people fought tooth and nail to get it to not fail the build, because they didn't like that their new feature, which didn't introduce any vulns. per se, was failed. Later, other people did much gnashing and wailing over being "confused" because the yarn audit step was red-X, and they were confused as to why it was breaking on their feature branch. It wasn't: it was failing, but marked to be ignored, so the build continued. Other, required steps, were failing. (And were also marked with red Xs.) This was "all too confusing", and it was petitioned for yarn audit's CI step to just "fail" with a green checkmark. You can imagine, I'm sure, how many vulns. were addressed after that point.
I've also worked with devs who are just amazing, and aside from being brilliant in how they architect their code/systems in the first place, … they have a definite tendency to view lint errors as just "oh, oops, silly me fixed" and just Get It Done.
QC is hard.
(Regarding our audit step, too: we also had an ignore list. You could just add the vuln. to the ignore list, and that would technically placate CI. Obviously not the best … but CI didn't demand that you fix whatever the issue was.)
Well, here is the problem this introduces. Who is supposed to fix this and which PM's bucket of time does it come out of? This implementation just makes it seem like the answer is "whichever team is unlucky enough to make a PR when it is discovered." The perverse incentives of that are enormous.
> You can imagine, I'm sure, how many vulns. were addressed after that point.
This tells me that the responsibility isn't specifically assigned. This seems to be the broader issue, with the audit in the pipeline a managerial attempt to get vulns somehow fixed without doing the work to figure out how they will be fixed and who will do the work.
Someone actually needs to be responsible for it and be given time to do the changes. It seems like this was not done.
The team of eng responsible for the code against which a vuln. has been detected, would be my answer.
> This tells me that the responsibility isn't specifically assigned.
Well, no, for the very reason you've highlighted: PMs will be damned to have that time come out of their bucket. That is a problem, and I rather doubt this subthread'll solve the idiocy that is PMs.
Nonetheless, if we accept that vulns. are something the industry should pay attention to, i.e., that "keep stuff using secure versions of dependencies" is actually a best practice the industry should adhere to, someone has to triage the audit report, and then someone yet has to do the deed of correcting it.
Yet both your comment & a sibling have advocated the position of "no, drown the test result, don't block builds". If builds aren't blocked, there is zero impetus towards ever fixing vulns., so they don't get fixed, ever.
You're missing the larger point of my comment, too: there is no sensible setting for that CI step. Say I don't care about what the CI step does, say I don't attempt to take a position in this idiotic debate. I can't: literally any configuration boils down to one of two cases: a.) we're flagging the problem, and devs don't like that and whine and complain or b.) we're ignoring the problem, and security people claim they don't like that, and whine and complain.
I think you're also forgetting how software is actually developed in the wild by many companies: this example, like so many, use a "monorepo". I don't like this (I'd argue for single-purpose repos, and it'd be obvious who the owner of a vuln. is, since the whole repo is only owned by one team.) but nonetheless I'm pretty consistently outvoted on this point, and the majority of firms out there seem to go the monorepo route. So, in a monorepo, "who" is responsible for a vuln. against a third-party dependency? (It can be figured out, but the answer is certainly not as straight-forward anymore!)
(Single-purpose repos aren't without issue here either: often I find in that case that establishing these sorts of "common" CI steps to be hugely problematic: you sort of have to c/p a lot of code between repos, keeping that step's code updated is then a PITA, and many CI systems lack good support for "use this common step, perhaps not even located in this repo".)
Well... yeah? They're working on a feature and get arbitrarily told that they're not allowed to merge the feature until they go fix some completely unrelated vulnerability problem. How is that reasonable?
I don't think how you can avoid that unless you find people that have similar ideas to you about how to write code.
But it doesn't make the problem any less real. At my current job, I think our tests are really bad because they mostly don't test what really matters (also due to too many things being spread out across several services) and instead are coupled excessively to implementation details. That makes development slow and also means I can never be sure that my changes don't break something important.
When people say things like I this I always point out that this may be true for the first 2-5 years of you developing but if it's true 15 years in you're not actually improving you're just being mercurial.
At some point your growth as a developer moves outside of code and you recognize that there's beautiful code, devilish code, and then a huge grey area where things are just fine even if they aren't how you would have personally implemented them.
Out of interest, what kinds of things would you consider "beautiful" or "devilish"?
devilish, I once came across where someone had accidentally written a FSM that used http redirects as the transitions between nodes. All to animate a progress bar for a job queue. It was so ridiculous it honestly took me longer than I care to admit to understand what I was seeing, who the hell expects to see that?
OTOH, we have a codebase at my current employer that opens up a TCP socket to a service and passes pipe delimited strings. And you know what? I give the thumbs up to that. Why? The service it's communicating with is an old C++ service that was written around the time that XML started coming onto the scene. That's how you did things back then and it works perfectly fine, albeit it's probably not the most familiar to younger developers.
As new people are onboarded, stick with those rules but have periodic moments where you solicit feedback from your team on the approach and make changes.
It really does help to have a team agreeing on these things collaboratively, and ideally having a more senior person enforcing it during code review where it's reasonable.
There’s some related thoughts on safety practices that would be readily apparent to someone on a job site that apply to software as well. In my personal experience, people who don’t take pride in their work are the same ones that end up creating unsafe situations. The software parallels are easy to imagine.
The Salesforce tower in San Francisco begs to differ: https://www.popularmechanics.com/science/a29329189/salesforc...
Is it really that, or are there just so many parties involved, that a heavy bureaucracy/coordination layer is required to keep the project from collapsing on itself, and this happens to manifest as everything looking neatly organized and structured?
But to the OPs point, there is a stark difference in the quality of work between tradespeople who respect their craft and those who look at it as merely a means to a paycheck.
New business or acceptance criteria, architectural changes, and other human created obstacles.
It's like a skyscraper. The critical structural stuff is fixed but the architects and engineers try to minimize those pieces and create flexible spaces that can either be high density cubicles, enclosed offices, a cafe, etc...
Eventually it's true that you have to tear it all down and there's nothing wrong with that - a good developer will know when that makes sense based on the needs of the stakeholders and make that recommendation.
For me, pure functions and enforced types do the trick.
Not all code is created equally, not all code needs to be clean or have a lot of time spent on it. Many times those who "care" about clean code have difficulty understand when it is, and isn't appropriate.
"Clean code" is useless when it's an artistic aesthetic, but operationalizing the sense of "everything has a place, clean up as you go" has concrete value. You can spot teams that DGAF vs. teams that treat their responsibilities like a 5-star kitchen. In both cases, it's "just" a job, but that doesn't equate to carelessness.
It's about where you are at relative to the median in the group, typically, not about any absolute standard.
I disagree. I'll take a bet that given 10 experienced software engineers (we can define what this means -- title, years of experience, familiarity with language idioms, etc.) they'll mostly agree on samples A, B, C, ... of code if it's "clean" or not.
Code doesn't have to follow my preferred design pattern to be clean. The bar is don't be shitty, and I'll know shitty code when I see it[1].
Modern C++ codebases that make extensive use of templates, inheritance, exceptions, type coercion, closures are considered clean by many because the code looks nice. But code like that can be a nightmare to work with, because of long compile times, spooky action at a distance, etc.
Take these two files from LLVM:
https://github.com/llvm/llvm-project/blob/main/libcxx/includ...
https://github.com/llvm/llvm-project/blob/main/libcxx/includ...
Is this "clean code"? I'm confident there will be absolutely no consensus.
Don't you know a bad API, variable name, or architecture when you see one?
Why do programmers hate making those changes? They always require tossing out a lot of existing code. So I guess I could say the antagonist is inertia. Certainly has been my experience interfacing with vendors and BigCo developers. If you have the ability to suppress this feeling, to delete stuff that works but is not correctly coupled to the new problem, you will thrive regardless of the architecture.
I work on a team now where the three main contributors (including myself) all have _reasonably_ different programming styles (naming, whitespace, structure, const/immutable, algos). Having read enough code from the other two: I am confident they are good programmers, but their style is different from mine. Still, we try very hard to accommodate each other's style. When we make changes, we try to stay with the current style in the class / method. It has been a real learning experience for me.
codestyle enforcement via pre-commit hooks or CI build failure is the way to go. Trying to adpat to codestyle is a distraction. All distractions should be removed when coding.
The sibling comment that talks about "how quickly they can understand and use your code, and whether they can make changes without worrying about breaking things" is on the right track.
Be careful not to conflate clean code with premature optimization. Clean code is easy to read and follow and works in obvious ways, it isn't necessarily the fastest or most efficient code and nor does it mean it has to use deeply nested knowledge of how the system works to function in ways that are non obvious to new readers.
If everyone merely accepts good work silently, but talks about bad work all the time, then political focus within the org will shift to bad teams and bad people. At the extremes I’ve seen this result in the worst people getting promoted to the highest positions because they were infamous. That’s the same as being famous.
Think about Trump: he got elected because nobody could shut up about him, so to a lot of voters didn’t know anything about any other candidate. They voted for the one they recognised.
“He may have his flaws, but he’s not that bad.” is something I’ve heard at work and in the public sphere.
You’re immune to this effect, you’re about to say?
Name five good things that Hillary has done.
You got me absolutely confused here. There are 5 good things she had done? I think this proves that a bad alternative can get anybody elected.
She’s worked in public service for decades and has done many good things in that time. Most people can’t name any.
Everybody in the world can name five (fifty!) bad things Trump has done.
That’s why that man lives rent free in our heads. That’s why everyone knows his name. That’s why he got many of his votes: he’s recognisable.
This effect has nothing to do with any specific politicians.
It happens in workplace settings also. The quiet achievers get overlooked. The loudmouth screwup gets attention and even promotions.
I’m old enough to have seen this play out over and over again at every scale.
The point is not whether Hillary is good or bad.
The point is that most people are unable to remember good things that even very famous career people have done, despite this information being in the public record. It's just not covered, repeated, and as memorable as bad things that people have done.
You're focused on "Hillary bad!", not realising that that's the point I'm making. You can only remember bad things.
The exercise is to see if you can name good things. You can't, not because she's unadulterated pure evil, but because good things are not as memorable in general.
Try for a moment to rise above your primitive animal tribal instincts and think like an anthropologist.
I'm not even American!
I picked an example at random that I knew everyone would be familiar with. I have no skin in this game and who won or didn't win isn't the point. The point is the rules of the game.
Which proves the point you have no idea what you talking about. Go ahead then and list 5 great things she did.
This sounds true. Some people do the boring job of cleaning code and keeping the system reliable never got a praise. But some people, who never care about the quality of the system, fight the incidents and are called "hero".
> Don't get me wrong, people will expect you to write good and clean code. Still, you will rarely get any praise for it. Except sometimes from your colleagues who will review your code.
If I can read your code, I don't give a frog's butt if it's up to the nose in the air crowd's standards. That just isn't what is important to a business that actually wants to survive and make money. It's crazy, those same blog heads are the ones who are always so quick to advocate for a "total rewrite" every 6 mos. They think all this nonsense makes them Pros.
To add to this, people will have different opinions on what is "clean". In fact you will have different opinions on what is "clean"!
I've lost count of the times I've written some code, only to come back the next day and re-write it into a "cleaner" version.
Like writing an essay, it takes time and iteration, and sometimes there just isn't a stable solution. This has somewhat helped me in just writing the damn code (but my ADHD brain still habitually reaches for "perfection" over "get it done")
Of course this is the exception rather than the rule. I've done a lot of revision on my code. If you don't look back on your code and think of ways it can improve then there's a good chance you're not learning.
I say this exact same thing to junior engineers who are acclimating to code reviews and might be suffering a bit from imposter syndrome.
Talking with team members to get everybody to agree on what the style is can fix this.
Talking seems to not happen, in my recent work experiences - instead it's a pain to talk to people because you have to do fucking zoom meetings and it has to be scheduled. Or the company actively discourages the team from setting their own technical goals of quality in the name of some ridiculous abstracted version of addressing customer interests. It's out of balance - customer interests are left on the ground while quality is also left on the ground.
Having a tight team is what we should be focusing on! ChatGPT is going to eat so many of our jobs. We should be focusing on being excellent humans.
I was warned about that ego driven development while I was at school by the teachers. Apparently it's been formalized as gospel ... people lie to themselves calling it clean. Basking under the warm smile of Uncle Bob. Creep.
Yes, readable code is the winner. Not "ultra compact" or "so elegant you can't understand it". I once refactored an elegantly written but heavily opinionated codebase for a large feature - and my goal was to hand it off without any downstream complaints - so I doubled the LoC and the team that received it was very happy with all the non-terse logic and detailed comments (good code is self-explanatory on how it works, but not often on why it's there)
I'm talking about makefiles, build systems, shell scripts, packaging systems, installers and more.
I also think clean code is a luxury. The more your codebase is alive, the more likely that upgrades, features and fixes will go across the grain.
It's not like people are slobs, it's more like a living house where busy people come in out of the rain or snow, or put dishes in the sink for later, or occasionally spill a plate of food on the floor and have to clean it.
The ultimate proof of this is that the new AI models are VERY useful, but are stored as blob more similar to grey matter than structured modular code.
This is true most of the time, but assuming your boss and coworkers are doing their jobs well (which is why it's not true most of the time) people will regularly communicate the things that are helping or hindering in their work.
Of course, you might find that your perfectly clean code isn't as helpful as you expected, depending on what you see as "clean." You'll learn that people care about how quickly they can understand and use your code, and whether they can make changes without worrying about breaking things. That's all they care about, and that's what "clean" is supposed to serve and accomplish.