Hacker News Reader

Comment by usrbinbash | Hacker News Reader

Just my quick list

Code that relies alot on pointless abstractions (aka. bad "signal to noise ratio").

Code with lots of microdependencies to perform really trivial tasks.

Code with premature "optimizations" (usually done before actual performance measurements).

Code sacrificing readability (and thus maintainability) to be overly clever or to do trivial optimizations (usually with little to no impact on overall performance).

And of course all the different ways to mess up comments, style and versioning (Yes, I consider the versioning part of the code):

    * Uncommented code
    * Over-Commented code
    * Comment-Banners
    * Commented-Out code (git exists for a reason)
    * Inconsistent naming
    * entirely_too_long_variable_names for things that exist for 20 LOC
    * Unhelpful commit messages (my favorite: "fixed: some issues")
    * Bulk-Commits that change/fix/introduce multiple features or issues at once 
    * Missing tags
    * No semantic versioning

View on HN

> Code with lots of microdependencies to perform really trivial tasks.

"trivial" measure highly relies on programmer's/developer's experience and knowledge. For first developer traversing a tree and applying some updates based on nodes conditions would be "trivial" so thee places all the code in one function. For second developer the same problem would not be such trivial, so thee creates two functions "fn listFromNodes<T, U>(t: Tree<T>, pred: (fn(e: T) -> bool), map: (fn(e: T) -> U)) -> List<U>" and "fn applyUpdates(updates List<Update>)".

> Uncommented code

Why comment a code that's mapped to problem trivially observably (ex. if it's code describing some GUI component with style fields and your task is change style of its part)? I prefer commenting code that contains complex logic (which is measured through code reviews, developers skills, calls etc.) and its effects aren't observed obviously.

> Commented-Out code (git exists for a reason)

Yes and no. Commented-out code is usually in-place saved draft of old code. If some bug is encountered with new code, you can comment-out new code and uncomment-out old code quickly to discover a bug in new code later. Git is less easy to accomplish this task.

> Inconsistent naming

Some rudiment not quickly refactored, may be involved. Example, I had a project when I had type "CardRef" which initially contained only a name (string) of card which was used as a reference for it. Then I decided to include some scores for a card to selecting cards smarter. I kept a name for type the same because it was used a lot in a project for related entities.

> Why comment a code that's mapped to problem trivially observably

"trivially observable" is not a defined measurement however. What a piece of my code does may be perfectly obvious to me, but mystifying to someone else, or even to myself in 4 months. I am not advocating for half a page of comments before each top level definition, or trivial comments like `// this requests data from server` , but a single line of comments above each function or typedef isn't too much to ask.

> If some bug is encountered with new code, you can comment-out new code and uncomment-out old code quickly

I can also check out the commit where it was last changed, stash the info, and apply it to my dev branch. Same functionality, and less cruft in the code.

I really think this list is too low level. What matters a lot more is how the data structures and architecture is designed. I can easily imagine a nightmare of a system that follows all the rules in the list. And I have worked on great solid maintainable code that didn’t follow that list at all. Also, semantic versioning doesn’t work. Developers typically have no clue whether a small change might make an update incompatible. Fixing a bug might make your code incompatible without you knowing it for example (since software using your code has fixes to fix your bug).

Basics matter.

Let's say the architecture of a house is beautiful, and its a marvel of structural engineering, but; If the floorplan is unintuitive, the stairwells alternate between 3 different parts of the building each floor, none of the rooms are labeled, all the access shafts are of different size, the original architect didn't even keep a copy of a blueprint, and some rooms are simply blocked off with a sign on the bricks saying "maybe need that later"...

...then the guy who has to rewire the cables in the damn thing later is gonna have a bad day, and the guy who has to pay him for all the extra hours is not going to be happy either.

Most of the issues you are discussing are architectual issus. Architecture is not just how the house looks. An architecture also decides on the floor plan, staircases etc. Also, houses are not a good anology for software. So I caution against drawing any conclusions from your perception on how houses are constructed and the building industry in general.

Houses are a great analogy, especially when it comes to my argument about pointless abstractions.

A house where the floorplan is decided before the needs of the tenants are even clear, will likely have to be reshaped later. The same happens in overarchitectured codebases, eg. a dependency injection for some subsystem set up which will never have more than one type to depend on, making the code less obvious, less readable, and harder to maintain.

> pointless abstractions

Of course anything that is "pointless" is indeed pointless. And "great abstractions" are indeed great. What's your point? And what has that got to do with your origial low level list?

> A house where the floorplan is decided before the needs of the tenants are even clear, will likely have to be reshaped later

Yes obviously. And a well thought out floorplan that takes into account the needs of the tenants is great. Again, what's your point? And what does that have to do with your original low level list?

My point is: "Basics matter, Readbility counts."

And how is my list "low level"? It concerns both "low level" issues like missing comments or inconsistent naming, and "high level" architectural problems, like introducing pointless abstractions.

Both are code smells , and thus a valid answer to the topic.

entirely_too_long_variable_names -> I prefer very long variable names which are self-explaining and easy to grep. Abbreviations make code less consistent and less greppable. Also what's the point of making shorter variable names if you need a comment to explain what it's doing - or worse I need to email the author to find the answer. A variable name should be the minimal representation of the role of this variable, but without cryptic shortcuts or abbreviations.

I think the common denominator is, code that is harder to read and understand than it needs to be. (That also makes it harder to maintain and update than it needs to be.)

> Commented-Out code (git exists for a reason)

I tend to agree, but with some uneasiness.

I don’t often review the previous commit diffs when working, if I don’t know it is there why would I find it in git?

Reply on news.ycombinator.com