You use nice words to excuse bad code
TL;DR: Don't excuse bad code. Write a clean one!
Problems
- Readability
Solutions
- Rewrite the code and delete the comment
Context
The term comes from Martin Fowler's book "Refactoring: Improving the Design of Existing Code"
Sample Code
Wrong
# This is a function that adds two numbers
def s(a, b):
# Now you are going to add a and b
res = a + b
# And return the result
return res
Right
def sum(adder, anotherAdder):
return adder + anotherAdder
If you ask ChatGPT to improve this version it will actually worsen it:
def calculate_sum(number1, number2):
# Calculate the sum of two numbers
result = number1 + number2
return result
# In this improved version:
#
# The function name calculate_sum is more descriptive than sum,
# making it clear that this function calculates the sum of two numbers.
# (Wrong) it is more imperative and mistakes the 'what' with the 'how'
#
# The parameter names number1 and number2 are more meaningful
# than adder and anotherAdder, helping to indicate the purpose of each parameter.
# (wrong) They indicate type instead of role
#
# The comment # Calculate the sum of two numbers provides a clear
# and concise explanation of what the function does,
# making it easier for someone reading the code to understand its purpose.
# (wrong) in fact, it is an example of deodorant and useless comment
Detection
[X] Semi-Automatic
Most comments are code smells.
You can remove deodorant comments and improve the code.
Exceptions
- Comments should only be used to describe important design decisions.
Tags
- Comments
Conclusion
Remove any meaningless comment you find in your code.
Relations
More Info
Disclaimer
Code Smells are my opinion.
Credits
Photo by Ana Essentiels on Unsplash
The reason we mention comments here is that comments often are used as a deodorant. It's surprising how often you look at thickly commented code and notice that the comments are there because the code is bad.
Martin Fowler
This article is part of the CodeSmell Series.
Top comments (13)
In general, comments describing anything else than the copyright or the non-obvious reasoning behind the code should be avoided.
Agree but I also found that if I use comments for "non-obvious reasoning behind the code" I found that these should in contrast be as precise and detailed as possible. Saved me lots of time when looking at older code after a year.
Plus: If comments are rarely used and then this one comment block is long and detailed it might be of importance to read :-)
Every rules has its exception.
Unfortunately, if the method is already part of exposed API used by your customers, you cannot simply rename it. Either you deal with it and add a comment explaining what it actually do, either you go through painfull deprecation process.
The best is to catch it before it actually go into the wild. Code review is your friend here !
I disagree that "adder" and "anotherAdder" are good param names. "Adder" is a specific term in computer science/electronics, referring to an ALU component that performs binary addition. Given it would be unlikely to refer to that in a high-level language like python, I'd assume instead it was either a function or an instance of a class that performs addition. A number passed as a param isn't an active agent, as implied by "-er" suffix — it doesn't add itself, rather addition is performed on it.
Apparently "augend" and "addend" are sometimes used to refer to the 2 operands in addition, but IMO those are far too academic for general-purpose usage. "number1" and "number2" are easy to understand in the context of summing, and the function name
sumtells us what's going to happen to them.I think you may have partially missed his point that the word "number" in the variable name specifies the type of value rather than its purpose. Types rarely, if ever, belong in variable names. In Python, they don't necessarily need to be "numbers" with operator overloading. They could be, but they could also be objects of some matrix class, or something else that makes sense to sum.
Using the actual name, "addend", for the arguments of an addition would be better (he just had the name for it wrong but was clearly his intention). However, going with your point, the function name does make it clear what it does. So more generic names for the parameters might be OK. But in that case, just go with something like
xandy, or maybex1andx2.I agree about not including types in variable names as a general point, but in some cases there isn't much more specific you can say about them (without making them overly verbose or confusing).
But I agree with you that
xandywould be fine in this case too, assumis such a common and well-understood operation.'x' and 'y' are amazing for math. not for computer science. IMHO
maximilianocontieri.com/code-smell...
In general, I agree with you that 'x' and 'y' are almost always poor variable names. In this specific example, the
sumfunction is quite literally a function in the math sense. You obviously went with that for the simplicity of the example. The concern @lionelrowe has is that the more descriptiveaddendmay be too obscure for the less math-oriented, so althoughxis also mathy, it may be more easily understandable in this context than a technical math term likeaddend.I also frequently use variable names like
x,y,a,b, etc. as params to short anonymous functions. Sometimes the variable is just super obvious from context (especially when you factor in TypeScript/intellisense). Verbose variable names are good when they help clarify, but bad when they just add noise.Computer science was born from mathematics. In math, the assignment of single-letter variables (i, j, x, y) is a good practice. The concept of reference arose from the variable and many people have wondered why mathematicians can work with such short variables, while computer scientists cannot. For mathematicians, once entered into a formula, variables lose all semantics and become indistinguishable. They have a lot of naming conventions and think very carefully about their names. The difference is that mathematicians always have a fully and formally defined local context, where one letter is enough to distinguish the variables.
learning.oreilly.com/library/view/...
Yeah, I was thinking the same thing and I found the following on wikipedia:
I would've liked to see the params named addend1 & addend2 or summand1 & summand2 or something.
I like your example of how ChatGPT takes the smell-free version and introduces at least 3 smells into a simple function.
Thank you.
We need to be in control and don't trust them blindly