Why Code Snobs Are Invaluable

http://mjswensen.com/blog/2015/01/30/why-code-snobs-are-invaluable/

by ignaces • 11 years ago

69 55 11 years ago

What about type theory weenies? The title is also a little link-baity. The person commenting on his pull request was not being a snob. He was making sure there was architectural coherence across the board. When codebases grow large having an idea of the overall architecture becomes an invaluable resource and the more consistent the codebase the easier it is to make sense of the architecture.

jeremiep4• 11 years ago

Type theory can help you build the proper abstractions to help the codebase grow and scale in a clean and manageable manner. It doesn't matter if you're working in Haskell or JavaScript either; types are still present and affect how well the different parts of the system can be composed together.

gknoy• 11 years ago

  The person commenting on his pull request was not being a snob. 
  He was making sure there was architectural coherence across the board.

This is actually the essence of the article. When people criticize things in our code that we see as "trivial", it's EASY to mistakenly ascribe that to snobbery or being a jerk. The entire point of this article is that we should look for the value that our peer is trying to add by pointing out inconsistencies or better ways of doing things.

Some of the best pull request comments for me have been the ones that made me say, "Dammit! .... you're right." Many of those PR comments have been replaced by impersonal linting tests that do the same, but the net result is still that I feel proud of what I write.

codygman• 11 years ago

What about type theory weenies?

Are we first graders?

serve_yay• 11 years ago

My experience tends to agree. A lot of things done in the name of not being a snob are really just laziness, the bad kind. And I know my own act shaped up almost instantly when I switched jobs and found myself surrounded by such folks.

picks_at_nits3• 11 years ago

Consider a colleague who constantly objects to non-idiomatic variable name casing (snake_case_in_ruby, camelCaseInJavaScript, dashes-in-lisp, scheme-predicates?, &c.)

When you first get a comment like this, it may feel like friction. After all, the code works. The cost of holding up a deploy and changing your code feels all out of proportion to the benefit. And it’s easy to extrapolate this: “If I get this objection to every pull request, I’ll never get anything done!”

But you don’t need to get this objection to every pull request. If the objection has some merit, no matter how trivial, you can change your naming, and never hear this objection again.

And the entire code base will be a little bit better. Your change is for now, a little bit better is forever, and everyone changing the way they name variables to be a little bit better adds up to a lot better.

And so it is with other forms of code snobs. Although it may feel like it’s not worthwhile to change the code today, if it’s a meritorious objection, then the right thing to do is change it and learn to write it that way the first time around, to develop a habit of writing it that way.

And then everyone will end up being a little bit better. And with every new commit, the code will get better. And that adds up like compound interest into making the code a lot better.

taeric4• 11 years ago

What about the colleague's who have successfully enshrined non-idiomatic variable naming and space rules in a project?

They seem to have just as much of this argument on their side, as far as consistency and such. Yet I actually loathe the decisions, as it causes a ton of friction with standard tools.

picks_at_nits3• 11 years ago

I stand by “if the objection has some merit.” If you don’t think it has merit, deploy the counter-snobbery.

Naming seems cut-and-dried, but there are other cases that aren’t so clear cut. What if there are two factions in a team, one of whom are very OO, the other are very functional?

ryandrake• 11 years ago

The norm seems to be: Conform to what's currently in the code.

Whether it's variable names, your brace style or indenting lengths, it doesn't matter if you think your way is the best, what matters is fitting in with the existing (shared) codebase so it's readable to everyone who has been working on it for the last 5 years.

Same is true for non-style things. If your project is 100K lines of 1998-era "C-with-classes" C++ code, your modern, templated, header-only, lambda-using work of art is going to cause more friction than it helps.

pekk2• 11 years ago

This kind of policy has to be set up front (or at least on a going-forward basis) or it'll never happen. You obviously can't fix this, all you can do is look out for yourself.

Circles of people who all agree to do something wrong and reinforce each other in it are one of humanity's big problems...

noblethrasher• 11 years ago

Everything causes friction with respect to something. So you have to examine the merits of the argument.

I, for instance, loathe the prescription that the names of .NET interfaces begin with "I". This is because I realized that once you understand the difference between a “class” and a “type”, then it becomes obvious that everything is an interface (abstraction is just naming things). But, the .NET style guide hinders that understanding since it enjoins you to focus on the little things (classes and interfaces) instead of the big things (types)[1].

So, “code snobbery” is about figuring out what you really want, and then creating cognitive affordances (or eliminating cognitive hinderances) with respect to that.

[1] This is especially apparent when you consider how interfaces and classes are defined in F#.

sanderjd• 11 years ago

I find that with naming style, it's hard to go the non-idiomatic route and stay consistent, because you are inevitably interacting with the standard libraries, which are in the idiomatic style, so your non-idiomatic code quickly clashes. Then it's not an argument for one or another idiom, but one for consistency, which is easier to make.

jaimefjorge• 11 years ago

"And that adds up like compound interest into making the code a lot better."

Like inverse (or reverse) technical debt. Excellent way of putting why code reviews are important.

I would just add that while improving adds up, it shouldn't cover the fact that blocking the team because of camel case issues is not the best use of time. There are static analysis tools that are free and open source that can do this work.

We are spending between 1/5th to 1/10th of our time reviewing code[1]. Most of the times the disciplined have to carry the burden of being 'that guy' that always has something to say.

1: http://www.quora.com/How-much-per-day-or-week-do-engineers-s...

tottenhm• 11 years ago

"...non-idiomatic variable name casing (snake_case_in_ruby, camelCaseInJavaScript, dashes-in-lisp, scheme-predicates?, &c.)..."

That usage of "idiomatic" feels idiomatic to me. ;)

(I'd usually bite my tongue about such things, but your nic is picks_at_nits, and the thread is about the virtues of being pedantic...)

jemfinch2• 11 years ago

Singletons were perhaps the worst possible example of code snobbery: code snobs (real code snobs, as in "no true code snob") typically detest the singleton pattern as less maintainable, less testable, and less flexible than dependency injection.

Terr_2• 11 years ago

In the author's example it's about internal consistency, a bunch of other similar items are already singletons.

Better a consistent-but-suboptimal architecture than a random grab bag of varying quality.

jemfinch2• 11 years ago

A journey of a thousand miles starts with the first step. If you're committed to code quality, existing low quality code should not prevent you from doing the right thing with new code; on the contrary, doing the right thing with new code will free you to refactor or update the old code to be cleaner.

zaphar2• 11 years ago

Over reliance on singletons can indeed be less maintainable, less testable, and less flexible than dependency injection.

However sometimes a singleton is in fact more maintainable and improves the code massively. Nor do they have to be less testable when implemented well.

None of which detracts from the article which is about consistency in the code base. A knew jerk reaction to the singleton completely misses the point.

thedudemabry2• 11 years ago

I totally agree that the singleton detail is beside the article's point, but what is the modern case for singletons? It seems like singletons are now considered a synonym for global variables. What other purpose do they serve?

taeric4• 11 years ago

I'm fine for code snobs. I just despise swimming against the tide in tooling and formatting. If you care enough that the code look a certain way, make sure this way is a few keystrokes away for any developer using standard tools.

Then there are those that insist on highly flexible and overly engineered solutions. Yes, they got it to work. No, they aren't doing any favors to anyone that was wanting to pick up the code and make changes. (Been there, done that. On both ends.)

Basically, if you encounter a large amount of friction for every new contributor to your codebase. Consider that the source of that friction is not necessarily the "low quality" of the contributors.

picks_at_nits3• 11 years ago

Then there are those that insist on highly flexible and overly engineered solutions. Yes, they got it to work. No, they aren't doing any favors to anyone that was wanting to pick up the code and make changes.

https://en.wikipedia.org/wiki/Inner-platform_effect

pekk2• 11 years ago

Code review is really useful. People should listen to reviews of their code, even if they're arguable, for the sake of team harmony and having a system of checks and balances.

But a "code snob" is not necessarily correct just because he is a snob and sounds confident. There is sometimes a calibration problem where someone is trying hard to enforce practices which are actually wrong, or argues vehemently against things which are normal and okay. This is actively counterproductive and it would be better just to have less enforcement.

One way to check for this calibration would be to ask whether the snob is advocating something that will obviously benefit people beyond the snob objectively, e.g. fewer broken builds or adhering to a widely used standard style. A useful reviewer isn't just pushing his own agenda.

eldavido• 11 years ago

It depends a lot on your mentality toward writing code. I find matching the long/short-termness of engineering practice to the overall mentality of the organization results in the least friction.

Concrete example: if you're building a system to last 5-10 years, act like it. Spend time getting the architecture, the variable names, and the module structure correct. But realize not everything must be built to last. I read somewhere else on HN that some groups inside of Google plan a piece of code to handle 10x growth in traffic/storage/resource use before it's thrown out outright and rewritten -- so don't overdesign.

Maybe the real lesson is to invest in sufficiently good modularity that it's easy to throw things out without too much pain. Nothing lasts forever.

BurningFrog• 11 years ago

Good modularity IS good design and pays off in many other ways as well.

shanemhansen• 11 years ago

I can't find the reference but my rule of thumb is actually to build for 10x and design for 100x. That is to say if my service is now processing 1k requests/s I'd like the capacity for 10k requests/s and I'd like the existing architecture to be scalable by adding more nodes up to 100k requests/s.

gumby• 11 years ago

The last sentence is key: 'Perhaps a "code snob" is simply the term lazy programmers use to describe their more disciplined peers.'

If your colleagues really are your peers -- in that you trust each other, don't get pissed off when someone else makes a stupid comment, or when someone points out (hopefully politely) that you made a stupid comment -- then "code snob" is a joke term.

If you aren't lucky enough to have fallen into one of those high performance teams, then both types exist: the helpful code snob and the condescending one. The latter isn't help at all.

verbin217• 11 years ago

the helpful code snob and the condescending one. The latter isn't help at all.

HA! You're straight up wrong. You're never going to find someone more motivated to BE right. They don't care about appearing to be right for lesser people. If you have skin thick enough to tolerate them they'll be immensely valuable. Also, their attitude is a huge disadvantage in the workplace causing them to be far more exploitable (cheaper) than their peers.

wmil• 11 years ago

It depends what you mean by code snob... I've had a dev replace large section of my code with "cleaner" code that didn't work in all cases, comment out the failing tests, then push that.

Some people are convinced that making components isolated and aesthetically clean is more important than making them functional.

ultrasaurus• 11 years ago

There's a pretty large distinction between reviewers that are manually linting the code base and ones who change code to be easier to read but less working :)

logicallee• 11 years ago

I've had a dev replace large section of my code with "cleaner" code that didn't work in all cases, comment out the failing tests, then push that.

This is really interesting. Can you elaborate on what they might have been thinking when commenting out those tests? (which seems pretty blatant). Were the tests commented as to what they were doing, and obviously were important, relevant, and should have passed?

My impression is that when you go to a higher (cleaner) level of abstraction, you lose some immediate abilities. For example if you move a global variable without classes to a static variable in the class it really belongs with (after writing that class), you can never (by doing that) enable additional functionality over having it be a global variable. But the variable might no longer be accessible at some point, perhaps if a different base class has a static member whose initialization depends on another one already being initialized - that sort of thing.

which is the whole point. you are applying a new abstraction, and in the new abstraction they're not global variables but properties of a class. Perhaps some tests become irrelevant or need to be rewritten.

So, you could have some test fail as written, because you've moved to a higher level of abstraction. This makes me wonder what the exact tests were in the case you refer to...

nightski• 11 years ago

Well it would likely be better to have a discussion about it then? Instead of a brewing conflict among members of the team? It's also where leadership comes in. Someone needs to make a decision and guide the team in a particular direction. The most important thing is not that he/she chooses the "best" way (implying one engineer was "wrong") but rather simply that a way is chosen and everyone follows it.

RogerL• 11 years ago

I have to disagree, no bugs is better than some bugs. Pushing a change that introduces bugs is wrong (without the scare quotes).

This (sw design) is a multidimensional optimization problem with a somewhat vague cost function. But argmin on the # of bugs and argmax on feature completion is far more important than everyone following some random standard.

andhof-mt• 11 years ago

But your thinking like a programmer. Some times it is also valuable to think like a business.

What is the goal of this software? Ship now? MVP?

Are we trying to polish the software? Or just get it to work? Is it a prototype?

Some thoughts to take into consideration.

jeremiep4• 11 years ago

It could also be a double-edged sword. I've seen it happen more times than I can remember.

Management sets a clear deadline, we rush the code to get it done by its due date. Then the product sits there for months doing nothing while we're in between projects wasting time away. Half a year down the road we get asked for support because some things broke which would've been working fine had we taken the proper time to built them.

From personal experience, most businesses don't think in terms of maintenance and debugging. These are costs which can very quickly add up to way more than the saved development costs.

goshx2• 11 years ago

Half a year down the road we get asked for support because some things broke which would've been working fine had we taken the proper time to built them.

This seems to be so common. Almost every project seems to break or have issues at some point and the excuse is always the same: oh we didn't have time to do it properly. It makes me wonder if that is indeed always the case. This leads to a question... are there projects that are done in enough time so that they won't break?

j_s• 11 years ago

It is nice to be able to point to a static code analysis tool and/or project guidelines to shift the discussion away from anyone's interpretation of best practices.

joesmo• 11 years ago

Suppose that in the example given, changing the existing classes to not be singletons was just as valid as changing the new ones to be singletons. Or there was a good reason for the new classes not being singletons that the code reviewer did not understand. In these cases I'd call the reviewer a code snob. I don't think the reviewer in the example given is a code snob. A code snob to me is one who asks for or makes changes that have no benefit except to himself and might even be detrimental to the project. Since all changes do take time, the benefit of the changes absolutely need to outweigh the time spent making them for them to be valid.

Kiro• 11 years ago

OT but when do you need singletons in JavaScript?

ceslami• 11 years ago

Pretty much the same as in any other language. When you want to guarantee that there will only ever be one instance of an object.

It is a useful pattern for managing data synchronization (ie, if I only ever have one instance of a User, I know another instance won't mess up its state accidentally.)

richardlblair• 11 years ago

Where I work we keep on each other about nitpicky things, but when we do it, we declare it. You will frequently see "Nitpick: don't like the variable name. Maybe try <somethingLessCrappyThanWhatTheyWrote>"

And you know what? No one complains. If you don't want nitpicks, conform to the coding standard of the place of which you work. It is _their_ code after all.

klagermkii• 11 years ago

I certainly support any drive for code that is cleaner or more consistent, I just feel like for every good "code snob" there are a lot of "code snobs" that are basically just bikeshedding.

By that I mean that they don't know how to fix poorly structured code, suggest a better algorithm, or improve the interface... but they still want to feel and look useful, so they instead burn a lot of time finding relatively trivial things to change.

jquast• 11 years ago

I work very hard to have tools reject pull requests as much as possible. For python I am making good use of https://github.com/landscapeio/prospector to enforce pep257 and pep8 compliance. It also catches a lot of things (through pylint and frosted/flake8) that humans often do not, such as unused variables.

The advantage is that tools reject the pull request, and not a person. It doesn't harm ego to be rejected by a tool.

goshx2• 11 years ago

Well, I guess there is a need for a single definition of what a "code snob" is.

In OP's example, the singleton is a design pattern, not a coding standard. It should be used if it is applicable to the scenario, as it seem like it was.

Other people here in the comments are talking about "code snobs" as people that like to follow certain coding standards. IMHO, coding standards should be defined by the entire team and everyone has to follow. There is no way to have a consistent code without that, so it is not about being snob, it is about having a consistent codebase.

Now, if there are coding standards or an architecture defined but someone wants to use a different one because they want to be snob about it, then you have a problem. If there is a standard, let's say a naming convention as a simple example, and someone wants to change it at some point, you either have to change everything else to follow the same new standard, or don't start a new standard at all. Otherwise you are not resolving the original issue, you are simply making it worts by having more than one standard in your code base.

TEMPLEOS_DEV• 11 years ago

Depends on whether they have personality disorders.

Why Code Snobs Are Invaluable

Related Stories