this post was submitted on 05 Jan 2025
57 points (90.1% liked)

Programming

17752 readers
558 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities [email protected]

founded 2 years ago
MODERATORS
snowe
Ategon
[email protected]
57
Code Smells Catalog (luzkan.github.io)
submitted 2 days ago by [email protected] to c/programming
31 comments fedilink hide all child comments
top 31 comments
sorted by: hot top controversial new old
[–] [email protected] 1 points 15 minutes ago

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.

[–] [email protected] 23 points 2 days ago (2 children)

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.

[–] [email protected] 3 points 1 day ago

10 lines is a bit much, that's hardly more readable than one.

Then again, it depends on the language.

[–] [email protected] 8 points 2 days ago (1 children)

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.

[–] [email protected] 6 points 1 day ago (1 children)

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)

[–] [email protected] 4 points 1 day ago (1 children)

"Some people do a bad job commenting and updating comments, so lets not do comments" is not an approach that works for me.

Most of my code is at the prototype level. I'm concepting something out, usually paired with hardware.

If someone can't follow what I'm doing, its going to lead to problems. If a change happens to the hardware being controlled, code will not be good enough on its own.

Rather than being accepting of bad commenting practice, make comments (and updating them properly) part of good practice. In my experience, It saves time in the long run and leads to better code at the end.

[–] [email protected] 5 points 21 hours ago (1 children)

That's not what I said. I said that comments can often (but not always) be replaced with good and explicit names.

This can be pushed to some extreme by making functions that only get called at a single place in the code, just for the sake of being able to give a name to the code that's inside (instead of inlining it and adding a comment that conveys the same informations as the function's signature)

It's definetly not for everyone, but for beginners/juniors it gives something objective they can aim for when trying to build good coding habits

[–] [email protected] 3 points 14 hours ago* (last edited 14 hours ago) (1 children)

I am going to disagree, comments should be an explanation.

The code is what's being done, a comment should be why its being done.

[–] [email protected] 4 points 9 hours ago* (last edited 9 hours ago) (1 children)

I'm not sure how we disagree. At least, I don't disagree with you. My whole comment was talking about "what" comments. "Why" comments are a very good thing to have where they're needed

[–] [email protected] -1 points 8 hours ago (1 children)

Not updating comments with code is what I'm talking about - that's not a comment problem, thats a programmer problem.

If they aren't updating the "why", that programmer is the problem, not comments.

[–] [email protected] 3 points 8 hours ago (1 children)

When refactoring, it's often the "what" that changes, not the "why"

[–] [email protected] 1 points 7 hours ago* (last edited 6 hours ago)

That really depends.

Especially for a function that may see use in a variety of scenarios.

I'm going to be firmly against anyone suggesting against proper comments - which, I'm sorry, but you are by your own statement.

Code will change for many, many, many reasons beyond just refactoring.

Edit: and why it was refactored is important as well.

There are just so many reasons, and yes, I will continue to be against this newer trend of "dont comment, make codes your comments".

All that is, is a great way to make your code harder to manage later. It doesnt take much effort to explain why you're doing something.

[–] [email protected] 16 points 2 days ago* (last edited 2 days ago) (3 children)

Some of these mostly just someone's opinion, and I don't quite agree with all of them

[–] FizzyOrange 2 points 9 hours ago (1 children)

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".

[–] [email protected] 2 points 6 hours ago (1 children)

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.

[–] [email protected] 1 points 6 hours ago (1 children)

inheritance rarely solves anything

[–] [email protected] 1 points 5 hours ago (1 children)

You gotta know how to use it properly

[–] StrikeForceZero 1 points 25 minutes ago (1 children)

At that point I would argue composition/traits are the way to go.

"This extends Draggable". That's great but now we can't extend "Button" to override the click handler.

Traits: You wanna have Health, and do Damage, but don't want to implement InventoryItem? No problem. You wanna be an Enemy and InventoryItem? Go for it. What's this function take? Anything that implements InventoryItem + Consumable

[–] [email protected] 1 points 26 seconds ago

use an interface?

[–] Michal 3 points 1 day ago (1 children)

Not all smells are bad

[–] [email protected] 3 points 1 day ago

And focussing on what not to do is not the best way to get things right

[–] onlinepersona 0 points 1 day ago

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.

Anti Commercial-AI license

[–] DrDeadCrash 10 points 2 days ago (2 children)

I feel like if one tried to follow all of these "rules" at all times nothing would get done, at all.

[–] [email protected] 6 points 1 day ago* (last edited 1 day ago)

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.

[–] [email protected] 11 points 2 days ago

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.

[–] [email protected] 6 points 2 days ago (1 children)

half of these will make your code better lmao

[–] [email protected] 9 points 2 days ago (4 children)

which half?

[–] [email protected] 1 points 55 minutes ago

Like all things programming; It Depends.

[–] [email protected] 12 points 2 days ago

I'll never tell

[–] [email protected] 5 points 2 days ago

My code is exclusively Complicated Regular Expressions and it's screaming fast.

[–] [email protected] 4 points 2 days ago

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