4 ms·

"Code smell" is especially ambiguous and annoying.

by vlunkr 3y ago

DubiousPusher 3y ago
Is it though? Like, if I see a lot of nested "ifs" with "returns" sprinkled around, I'm getting "icky" vibes way before I intellectually know what is wrong. Like, I don't think you can get away with just typing "code smell" in a CR. That's lazy but the gut reaction to some code can be a useful starting point.
- fsckboy 3y ago
  > a lot of nested "ifs" with "returns" sprinkled around how do you feel about the code smell of a tail-recursive fizz-buzz?
  - francoisdevlin 3y ago
    Smells like breakfast
  - DubiousPusher 3y ago
    Erase it and do it iteratively like any sane person! Kidding aside, I'm more talking about the kinds of large procedures common to C++ codebases. These involve many sequential state changes which leave lots of opportunities for not initializing things and the like when the right set of conditions are met. The more paths available through this kind of code the more likely it becomes we'll create a bad one and fail to anticipate the conditions which trigger it.
  - tialaramex 3y ago
    If it's in a language with guaranteed tail recursion optimisation and it's idiomatic in that language (these two things often go together) then fine, seems reasonable, Go With God. Otherwise, nope, I don't care that "I checked and it's optimised correctly". We do not write optimiser dependent code in my house, write something that won't explode messily when we change target or upgrade the tools or whatever and now it stack overflows.
- opportune 3y ago
  I kind of feel this way sometimes, but some business or even non-business logic will require some inherent minimum amount of cyclomatic complexity. And there is only so much you can do to hide it or spread it around; depending on the specifics it may actually be better to put the whole ugliness in one place I guess that is why it’s a smell and not an anti pattern, but I work in a codebase with a lot of this kinda stuff and as much as I hate the complexity required to read it, I’ve been around long enough to know why all those ifs are there and even added a few myself.
eyelidlessness 3y ago
Intentionally ambiguous: the phrasing is specifically meant to suggest that it’s assumed non-universal and might not apply to all circumstances. If you embrace that ambiguity it’s a good antidote to the “considered harmful” problem and may well be less annoying.
- BulgarianIdiot 3y ago
  - "Bad smell" immediately invokes disgusting sensory experiences. It's intended to be ambiguous but it's not used this way. - "Anti-pattern" is also ambiguous, because patterns have some bad uses, and anti-patterns have some good uses. - And "Considered harmful" is also ambiguous. To consider something harmful is not the same as it being harmful, you're just considering it. These phrases are used automatically and intend to sound authoritative about something being bad, period, despite their historical nuance and ambiguity. And frankly they're all disgusting and annoying at this point. They show the author is more into clichés and clickbait, than substance.

"Code smell" is especially ambiguous and annoying.

by vlunkr 3y ago

"Code smell" is especially ambiguous and annoying.

Is it though? Like, if I see a lot of nested "ifs" with "returns" sprinkled around, I'm getting "icky" vibes way before I intellectually know what is wrong. Like, I don't think you can get away with just typing "code smell" in a CR. That's lazy but the gut reaction to some code can be a useful starting point.

fsckboy 3y ago
> a lot of nested "ifs" with "returns" sprinkled around how do you feel about the code smell of a tail-recursive fizz-buzz?
- francoisdevlin 3y ago
  Smells like breakfast
- DubiousPusher 3y ago
  Erase it and do it iteratively like any sane person! Kidding aside, I'm more talking about the kinds of large procedures common to C++ codebases. These involve many sequential state changes which leave lots of opportunities for not initializing things and the like when the right set of conditions are met. The more paths available through this kind of code the more likely it becomes we'll create a bad one and fail to anticipate the conditions which trigger it.
- tialaramex 3y ago
  If it's in a language with guaranteed tail recursion optimisation and it's idiomatic in that language (these two things often go together) then fine, seems reasonable, Go With God. Otherwise, nope, I don't care that "I checked and it's optimised correctly". We do not write optimiser dependent code in my house, write something that won't explode messily when we change target or upgrade the tools or whatever and now it stack overflows.
opportune 3y ago
I kind of feel this way sometimes, but some business or even non-business logic will require some inherent minimum amount of cyclomatic complexity. And there is only so much you can do to hide it or spread it around; depending on the specifics it may actually be better to put the whole ugliness in one place I guess that is why it’s a smell and not an anti pattern, but I work in a codebase with a lot of this kinda stuff and as much as I hate the complexity required to read it, I’ve been around long enough to know why all those ifs are there and even added a few myself.

eyelidlessness 3y ago

Intentionally ambiguous: the phrasing is specifically meant to suggest that it’s assumed non-universal and might not apply to all circumstances. If you embrace that ambiguity it’s a good antidote to the “considered harmful” problem and may well be less annoying.

BulgarianIdiot 3y ago
- "Bad smell" immediately invokes disgusting sensory experiences. It's intended to be ambiguous but it's not used this way. - "Anti-pattern" is also ambiguous, because patterns have some bad uses, and anti-patterns have some good uses. - And "Considered harmful" is also ambiguous. To consider something harmful is not the same as it being harmful, you're just considering it. These phrases are used automatically and intend to sound authoritative about something being bad, period, despite their historical nuance and ambiguity. And frankly they're all disgusting and annoying at this point. They show the author is more into clichés and clickbait, than substance.