Code Smells Catalog
Code Smells Catalog
A collection of Bad Code Smells in a Catalog form for Developers & Researchers. Code Smell is a typical bad code implementation, and learning these concepts immiedietly makes you a better developer!
I've been rallying against clever code for years!
Sure, it makes you have less lines for your l33t code solutions, but in the real world, it sacrifices the maintainability of code that others will eventually work on.
Between a clever 1 line fix and maintainable 10 line fix, I'll choose the 10 line every time.
As an extensive commenter, I completely agree.
I need to know wtf I was doing, making it convoluted to save a few lines is pointless.
It's often a good idea to make the code itself very explicit through verbose function and variable names, rather than writing comments that could lead to inconsistencies between code and comments (by not updating the comments at the same time as the code) (see "Fallacious Comments" from the catalog)
10 lines is a bit much, that's hardly more readable than one.
Then again, it depends on the language.
Some of these mostly just someone's opinion, and I don't quite agree with all of them
Not all smells are bad
And focussing on what not to do is not the best way to get things right
Agreed. Every time somebody links this to "prove" or underline their argument, I roll my eyes. There are a lot of subjective things there and many that are actually valid code.
Any specific ones? I've seen this before and I thought I would feel the same way as you before I read them, but actually the vast majority are pretty basic things that are not really arguable.
It's definitely nice to have a list like this to point inexperienced colleagues to in code reviews. It's a bit more authoritative than "trust me bro, I've written a lot of code".
To preface, I think it's best to focus on what the right approaches are. Not on what to avoid. And when you see a student making a mistake, showing them how a different approach is handier (if possible) is what I suggest you do.
Having something to point at doesn't help much
vertical separation
This one argues against organizing your code in a way that shares variables are in one place. There are arguments to be made either way, but normally you'd scope your variables in a way that the ones specific to a particular bit of code are not accessible from elsewhere.
null check
Suggest writing a custom class to do what most languages can solve with inheritance or even better: the ? syntax.
inconsistent names / styles
Yes, it can be annoying. No, clarity is more important than insisting on removing that extra underscore.
complicated Boolean expression
They're advocating the use of a function to replace an expression. Sometimes this works, but the task of a boolean expression is not always easily expressed in a couple words. And so you can end up with misleading function names. Instead, just put a comment in the code.
callback hell
Not even a code smell. It's an issue from back when languages like JavaScript didn't support promises yet, but callbacks were popular. Cose got hard to read with a little complexities.
I feel like if one tried to follow all of these "rules" at all times nothing would get done, at all.
A code smell isn't supposed to be automatically bad. A smell is an indication that something might be wrong. Sometimes using a smelly pattern is legitimately the only way to do something.
Apart from the fact that, as another commenter said, "smells" are not "rules", I think most of these points come down to developing good habits, and ultimately save a lot of time in the long run by initially spending some time thinking about maintainability and preventing/limiting technical debt accumulation.
half of these will make your code better lmao
which half?
I'll never tell
I'm not going through every one, but null checks, vertical separation, status variables and binary operator in name, are all things that often make your code better and more readable
My code is exclusively Complicated Regular Expressions and it's screaming fast.
Like all things programming; It Depends.
I think to present rules like this as hard rules, with little explanation and no nuance is harmful to less experienced engineers.
A prime example here is the Duplicated Code one. Which takes an absolute approach to code duplication, even when the book that is referenced highlights the Rule of Three:
The Rule of Three Here’s a guideline Don Roberts gave me: The first time you do something, you just do it. The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway. The third time you do something similar, you refactor. Or for those who like baseball: Three strikes, then you refactor.I've seen more junior devs bend over backwards, make their code worse and take twice as long to adhere to some rules that are really more what you'd call guidelines than actual rules.
Sure, try to avoid code duplication, but sometimes duplicating code is better than the wrangling you'd need to do to remove it.
Making extra changes also leaves extra room for bugs to creep in. So now you need to test the place you were working, and anywhere else you touched because of the refactoring.
Well it's in the name, they are code smells, not hard rules.
Regarding the specific example you cited, I think that with practice it becomes gradually more natural to write reusable functions and methods on the first iteration, removing the need for later DRY-related refactorings.
PS : I love how your quote for the Rule of Three is getting syntax highlighted xD (You can use markdown quotes by starting quoted lines with
>)The site doesn’t define what a code smell is, though. It’s just a list of Don’t Do’s.
That’s kind of the nuance I would be hoping for.
Something like:
Code Smells are clues that something is amiss. They are not things that always must be ‘fixed’. You as an engineer will, through experience in your own codebase and reading of others, develop a sense of the harm imparted by and the cost of fixing Code Smells. It is up to you and your team to decide what is best for your codebase and project.
(The rule of 3 formatting was intentional, given the community we’re in)
Ironically the site is glitchy, unresponsive and hardly usable on mobile.
Works fine for me. Which OS and browser are you using ?