The parameter may be well-named; it’s hard to tell without knowing the name of the function. I assume that the comment was written by the original author of the function, and it was a reminder of what passing false
into someFunction
means, but for anyone coming along afterward, it’s a little unclear at first glance.
Using a positive variable name (suggested in Code Complete) may be the simplest change that would make this snippet easier to read, e.g.
void someFunction(boolean remember);
then ourFunction
becomes:
void ourFunction() {
someFunction(true /* remember */);
}
However, using an enum makes the function call even easier to understand, at the expense of some supporting code:
public enum RememberFoo {
REMEMBER,
FORGET
}
...
void someFunction(RememberFoo remember);
...
void ourFunction() {
someFunction(RememberFoo.REMEMBER);
}
If you’re unable to change the signature of someFunction
for whatever reason, using a temporary variable makes the code easier to read as well, sort of like simplifying conditionals by introducing a variable for no other reason than to make the code easier to parse by humans.
void someFunction(boolean remember);
...
void ourFunction() {
boolean remember = false;
someFunction(remember);
}
Correct comment to put for boolean function arguments that are “false”?
From some open source projects, I gathered the following coding style
I always have doubt about what
false
means here. Does it mean “forget”, or does the “forget” refer to its corresponding parameter (like in the case above), and “false” is meant to negate it?What style is used most often and what is the best way (or some of the better ways) to avoid the ambiguity?
8
In the sample code you posted, it looks like
forget
is a flag argument. (I cannot be certain because the function is purely hypothetical.)Flag arguments are a code smell. They indicate that a function does more than one thing, and a good function should do only one thing.
To avoid the flag argument, break up the function into two functions that explain the difference in the function name.
Flag argument
No flag argument
edit: Ideally, you don’t need to keep around a function with the flag parameter at all. There are cases along the lines of what Fowler calls a tangled implementation where completely separating the functions creates duplicated code. However, the higher the cyclomatic complexity of the parameterized function, the stronger is the argument for getting rid of it.
This is only a hunch, but a parameter named
forget
sounds like feature envy. Why would a caller tell another object to forget something? There may be a bigger design issue.3
In layman’s words:
false
is a literal.false
someFunction
to not forgetsomeFunction
that the parameter forget isfalse
someFunction
to rememberIn my opinion it would be better if the function was like this:
the you can call it
or keep the old name but change your wrapper function to
EDIT:
As @Vorac stated, always strive to use positive words. Double negation is confusing.
3
The parameter may be well-named; it’s hard to tell without knowing the name of the function. I assume that the comment was written by the original author of the function, and it was a reminder of what passing
false
intosomeFunction
means, but for anyone coming along afterward, it’s a little unclear at first glance.Using a positive variable name (suggested in Code Complete) may be the simplest change that would make this snippet easier to read, e.g.
then
ourFunction
becomes:However, using an enum makes the function call even easier to understand, at the expense of some supporting code:
If you’re unable to change the signature of
someFunction
for whatever reason, using a temporary variable makes the code easier to read as well, sort of like simplifying conditionals by introducing a variable for no other reason than to make the code easier to parse by humans.2
Rename the variable so a bool value makes sense.
This a million times better than adding a comment to explain arguments to a function because the name is ambiguous.
2
Create a local boolean with a more descriptive name, and assign the value to it. That way the meaning will be more clear.
If you can’t rename the variable, then the comment should be a little more expressive:
2
There is a good article that mentions this exact situation as it refers to Qt-Style APIs. There, it’s called The Boolean Parameter Trap and it’s worth a read.
The gist of it is:
This is a bizarre comment.
From the compiler’s perspective,
someFunction(false /* forget */);
is actuallysomeFunction(false);
(the comment is stripped). So, all that line does is callsomeFunction
with the first (and only) argument set tofalse
./* forget */
is just the name of the parameter. It’s probably nothing more than a quick (and dirty) reminder, that doesn’t really need to be there. Just use a less ambiguous parameter name, and you won’t need the comment at all.One of advices of Clean code is to minimize number of unnecessary comments1 (because they tend to rot), and to name functions and methods properly.
Following that, I would just remove the comment. After all, modern IDEs (like eclipse) pop a box with code when you put a mouse over the function. Seeing the code should clear the ambiguity.
1 Commenting some complex code is ok.
2
To belabor the obvious, comments can lie. Thus is always better to make your code self-documenting without resorting to comments to explain, because some person (perhaps you) will change
true
tofalse
and not update the comment.If you can’t change the API, then I resort to 2 options
I like the answer about making the comment always true, but while good I think it misses the fundamental problem with this code — it is being called with a literal.
You should avoid using literals when calling methods. Local variables, optional parameters, named parameters, enums — how you best avoid them will depend upon the language and whats available, but do try to avoid them. Literals have values, but they don’t have meaning.
In C#, I used named parameters to make this clearer
Or
enum
:Or overloads:
Or polymorphism`:
The naming should always resolve ambiguity for booleans. I always name boolean something like ‘isThis’ or ‘shouldDoThat’, for example:
and so on. But when you are referencing someone else’s code, its best to just leave comment when passing values.
2
Filed under: softwareengineering - @ 09:26
Thẻ: boolean, coding-style, comments
Correct comment to put for boolean function arguments that are “false”?
From some open source projects, I gathered the following coding style
I always have doubt about what
false
means here. Does it mean “forget”, or does the “forget” refer to its corresponding parameter (like in the case above), and “false” is meant to negate it?What style is used most often and what is the best way (or some of the better ways) to avoid the ambiguity?
8
In the sample code you posted, it looks like
forget
is a flag argument. (I cannot be certain because the function is purely hypothetical.)Flag arguments are a code smell. They indicate that a function does more than one thing, and a good function should do only one thing.
To avoid the flag argument, break up the function into two functions that explain the difference in the function name.
Flag argument
No flag argument
edit: Ideally, you don’t need to keep around a function with the flag parameter at all. There are cases along the lines of what Fowler calls a tangled implementation where completely separating the functions creates duplicated code. However, the higher the cyclomatic complexity of the parameterized function, the stronger is the argument for getting rid of it.
This is only a hunch, but a parameter named
forget
sounds like feature envy. Why would a caller tell another object to forget something? There may be a bigger design issue.3
In layman’s words:
false
is a literal.false
someFunction
to not forgetsomeFunction
that the parameter forget isfalse
someFunction
to rememberIn my opinion it would be better if the function was like this:
the you can call it
or keep the old name but change your wrapper function to
EDIT:
As @Vorac stated, always strive to use positive words. Double negation is confusing.
3
The parameter may be well-named; it’s hard to tell without knowing the name of the function. I assume that the comment was written by the original author of the function, and it was a reminder of what passing
false
intosomeFunction
means, but for anyone coming along afterward, it’s a little unclear at first glance.Using a positive variable name (suggested in Code Complete) may be the simplest change that would make this snippet easier to read, e.g.
then
ourFunction
becomes:However, using an enum makes the function call even easier to understand, at the expense of some supporting code:
If you’re unable to change the signature of
someFunction
for whatever reason, using a temporary variable makes the code easier to read as well, sort of like simplifying conditionals by introducing a variable for no other reason than to make the code easier to parse by humans.2
Rename the variable so a bool value makes sense.
This a million times better than adding a comment to explain arguments to a function because the name is ambiguous.
2
Create a local boolean with a more descriptive name, and assign the value to it. That way the meaning will be more clear.
If you can’t rename the variable, then the comment should be a little more expressive:
2
There is a good article that mentions this exact situation as it refers to Qt-Style APIs. There, it’s called The Boolean Parameter Trap and it’s worth a read.
The gist of it is:
This is a bizarre comment.
From the compiler’s perspective,
someFunction(false /* forget */);
is actuallysomeFunction(false);
(the comment is stripped). So, all that line does is callsomeFunction
with the first (and only) argument set tofalse
./* forget */
is just the name of the parameter. It’s probably nothing more than a quick (and dirty) reminder, that doesn’t really need to be there. Just use a less ambiguous parameter name, and you won’t need the comment at all.One of advices of Clean code is to minimize number of unnecessary comments1 (because they tend to rot), and to name functions and methods properly.
Following that, I would just remove the comment. After all, modern IDEs (like eclipse) pop a box with code when you put a mouse over the function. Seeing the code should clear the ambiguity.
1 Commenting some complex code is ok.
2
To belabor the obvious, comments can lie. Thus is always better to make your code self-documenting without resorting to comments to explain, because some person (perhaps you) will change
true
tofalse
and not update the comment.If you can’t change the API, then I resort to 2 options
I like the answer about making the comment always true, but while good I think it misses the fundamental problem with this code — it is being called with a literal.
You should avoid using literals when calling methods. Local variables, optional parameters, named parameters, enums — how you best avoid them will depend upon the language and whats available, but do try to avoid them. Literals have values, but they don’t have meaning.
In C#, I used named parameters to make this clearer
Or
enum
:Or overloads:
Or polymorphism`:
The naming should always resolve ambiguity for booleans. I always name boolean something like ‘isThis’ or ‘shouldDoThat’, for example:
and so on. But when you are referencing someone else’s code, its best to just leave comment when passing values.
2
Filed under: softwareengineering - @ 09:26
Thẻ: boolean, coding-style, comments
Correct comment to put for boolean function arguments that are “false”?
From some open source projects, I gathered the following coding style
I always have doubt about what
false
means here. Does it mean “forget”, or does the “forget” refer to its corresponding parameter (like in the case above), and “false” is meant to negate it?What style is used most often and what is the best way (or some of the better ways) to avoid the ambiguity?
8
In the sample code you posted, it looks like
forget
is a flag argument. (I cannot be certain because the function is purely hypothetical.)Flag arguments are a code smell. They indicate that a function does more than one thing, and a good function should do only one thing.
To avoid the flag argument, break up the function into two functions that explain the difference in the function name.
Flag argument
No flag argument
edit: Ideally, you don’t need to keep around a function with the flag parameter at all. There are cases along the lines of what Fowler calls a tangled implementation where completely separating the functions creates duplicated code. However, the higher the cyclomatic complexity of the parameterized function, the stronger is the argument for getting rid of it.
This is only a hunch, but a parameter named
forget
sounds like feature envy. Why would a caller tell another object to forget something? There may be a bigger design issue.3
In layman’s words:
false
is a literal.false
someFunction
to not forgetsomeFunction
that the parameter forget isfalse
someFunction
to rememberIn my opinion it would be better if the function was like this:
the you can call it
or keep the old name but change your wrapper function to
EDIT:
As @Vorac stated, always strive to use positive words. Double negation is confusing.
3
The parameter may be well-named; it’s hard to tell without knowing the name of the function. I assume that the comment was written by the original author of the function, and it was a reminder of what passing
false
intosomeFunction
means, but for anyone coming along afterward, it’s a little unclear at first glance.Using a positive variable name (suggested in Code Complete) may be the simplest change that would make this snippet easier to read, e.g.
then
ourFunction
becomes:However, using an enum makes the function call even easier to understand, at the expense of some supporting code:
If you’re unable to change the signature of
someFunction
for whatever reason, using a temporary variable makes the code easier to read as well, sort of like simplifying conditionals by introducing a variable for no other reason than to make the code easier to parse by humans.2
Rename the variable so a bool value makes sense.
This a million times better than adding a comment to explain arguments to a function because the name is ambiguous.
2
Create a local boolean with a more descriptive name, and assign the value to it. That way the meaning will be more clear.
If you can’t rename the variable, then the comment should be a little more expressive:
2
There is a good article that mentions this exact situation as it refers to Qt-Style APIs. There, it’s called The Boolean Parameter Trap and it’s worth a read.
The gist of it is:
This is a bizarre comment.
From the compiler’s perspective,
someFunction(false /* forget */);
is actuallysomeFunction(false);
(the comment is stripped). So, all that line does is callsomeFunction
with the first (and only) argument set tofalse
./* forget */
is just the name of the parameter. It’s probably nothing more than a quick (and dirty) reminder, that doesn’t really need to be there. Just use a less ambiguous parameter name, and you won’t need the comment at all.One of advices of Clean code is to minimize number of unnecessary comments1 (because they tend to rot), and to name functions and methods properly.
Following that, I would just remove the comment. After all, modern IDEs (like eclipse) pop a box with code when you put a mouse over the function. Seeing the code should clear the ambiguity.
1 Commenting some complex code is ok.
2
To belabor the obvious, comments can lie. Thus is always better to make your code self-documenting without resorting to comments to explain, because some person (perhaps you) will change
true
tofalse
and not update the comment.If you can’t change the API, then I resort to 2 options
I like the answer about making the comment always true, but while good I think it misses the fundamental problem with this code — it is being called with a literal.
You should avoid using literals when calling methods. Local variables, optional parameters, named parameters, enums — how you best avoid them will depend upon the language and whats available, but do try to avoid them. Literals have values, but they don’t have meaning.
In C#, I used named parameters to make this clearer
Or
enum
:Or overloads:
Or polymorphism`:
The naming should always resolve ambiguity for booleans. I always name boolean something like ‘isThis’ or ‘shouldDoThat’, for example:
and so on. But when you are referencing someone else’s code, its best to just leave comment when passing values.
2
Filed under: softwareengineering - @ 09:26
Thẻ: boolean, coding-style, comments
Correct comment to put for boolean function arguments that are “false”?
From some open source projects, I gathered the following coding style
I always have doubt about what
false
means here. Does it mean “forget”, or does the “forget” refer to its corresponding parameter (like in the case above), and “false” is meant to negate it?What style is used most often and what is the best way (or some of the better ways) to avoid the ambiguity?
8
In the sample code you posted, it looks like
forget
is a flag argument. (I cannot be certain because the function is purely hypothetical.)Flag arguments are a code smell. They indicate that a function does more than one thing, and a good function should do only one thing.
To avoid the flag argument, break up the function into two functions that explain the difference in the function name.
Flag argument
No flag argument
edit: Ideally, you don’t need to keep around a function with the flag parameter at all. There are cases along the lines of what Fowler calls a tangled implementation where completely separating the functions creates duplicated code. However, the higher the cyclomatic complexity of the parameterized function, the stronger is the argument for getting rid of it.
This is only a hunch, but a parameter named
forget
sounds like feature envy. Why would a caller tell another object to forget something? There may be a bigger design issue.3
In layman’s words:
false
is a literal.false
someFunction
to not forgetsomeFunction
that the parameter forget isfalse
someFunction
to rememberIn my opinion it would be better if the function was like this:
the you can call it
or keep the old name but change your wrapper function to
EDIT:
As @Vorac stated, always strive to use positive words. Double negation is confusing.
3
The parameter may be well-named; it’s hard to tell without knowing the name of the function. I assume that the comment was written by the original author of the function, and it was a reminder of what passing
false
intosomeFunction
means, but for anyone coming along afterward, it’s a little unclear at first glance.Using a positive variable name (suggested in Code Complete) may be the simplest change that would make this snippet easier to read, e.g.
then
ourFunction
becomes:However, using an enum makes the function call even easier to understand, at the expense of some supporting code:
If you’re unable to change the signature of
someFunction
for whatever reason, using a temporary variable makes the code easier to read as well, sort of like simplifying conditionals by introducing a variable for no other reason than to make the code easier to parse by humans.2
Rename the variable so a bool value makes sense.
This a million times better than adding a comment to explain arguments to a function because the name is ambiguous.
2
Create a local boolean with a more descriptive name, and assign the value to it. That way the meaning will be more clear.
If you can’t rename the variable, then the comment should be a little more expressive:
2
There is a good article that mentions this exact situation as it refers to Qt-Style APIs. There, it’s called The Boolean Parameter Trap and it’s worth a read.
The gist of it is:
This is a bizarre comment.
From the compiler’s perspective,
someFunction(false /* forget */);
is actuallysomeFunction(false);
(the comment is stripped). So, all that line does is callsomeFunction
with the first (and only) argument set tofalse
./* forget */
is just the name of the parameter. It’s probably nothing more than a quick (and dirty) reminder, that doesn’t really need to be there. Just use a less ambiguous parameter name, and you won’t need the comment at all.One of advices of Clean code is to minimize number of unnecessary comments1 (because they tend to rot), and to name functions and methods properly.
Following that, I would just remove the comment. After all, modern IDEs (like eclipse) pop a box with code when you put a mouse over the function. Seeing the code should clear the ambiguity.
1 Commenting some complex code is ok.
2
To belabor the obvious, comments can lie. Thus is always better to make your code self-documenting without resorting to comments to explain, because some person (perhaps you) will change
true
tofalse
and not update the comment.If you can’t change the API, then I resort to 2 options
I like the answer about making the comment always true, but while good I think it misses the fundamental problem with this code — it is being called with a literal.
You should avoid using literals when calling methods. Local variables, optional parameters, named parameters, enums — how you best avoid them will depend upon the language and whats available, but do try to avoid them. Literals have values, but they don’t have meaning.
In C#, I used named parameters to make this clearer
Or
enum
:Or overloads:
Or polymorphism`:
The naming should always resolve ambiguity for booleans. I always name boolean something like ‘isThis’ or ‘shouldDoThat’, for example:
and so on. But when you are referencing someone else’s code, its best to just leave comment when passing values.
2
Filed under: softwareengineering - @ 09:26
Thẻ: boolean, coding-style, comments
Correct comment to put for boolean function arguments that are “false”?
From some open source projects, I gathered the following coding style
I always have doubt about what
false
means here. Does it mean “forget”, or does the “forget” refer to its corresponding parameter (like in the case above), and “false” is meant to negate it?What style is used most often and what is the best way (or some of the better ways) to avoid the ambiguity?
8
In the sample code you posted, it looks like
forget
is a flag argument. (I cannot be certain because the function is purely hypothetical.)Flag arguments are a code smell. They indicate that a function does more than one thing, and a good function should do only one thing.
To avoid the flag argument, break up the function into two functions that explain the difference in the function name.
Flag argument
No flag argument
edit: Ideally, you don’t need to keep around a function with the flag parameter at all. There are cases along the lines of what Fowler calls a tangled implementation where completely separating the functions creates duplicated code. However, the higher the cyclomatic complexity of the parameterized function, the stronger is the argument for getting rid of it.
This is only a hunch, but a parameter named
forget
sounds like feature envy. Why would a caller tell another object to forget something? There may be a bigger design issue.3
In layman’s words:
false
is a literal.false
someFunction
to not forgetsomeFunction
that the parameter forget isfalse
someFunction
to rememberIn my opinion it would be better if the function was like this:
the you can call it
or keep the old name but change your wrapper function to
EDIT:
As @Vorac stated, always strive to use positive words. Double negation is confusing.
3
The parameter may be well-named; it’s hard to tell without knowing the name of the function. I assume that the comment was written by the original author of the function, and it was a reminder of what passing
false
intosomeFunction
means, but for anyone coming along afterward, it’s a little unclear at first glance.Using a positive variable name (suggested in Code Complete) may be the simplest change that would make this snippet easier to read, e.g.
then
ourFunction
becomes:However, using an enum makes the function call even easier to understand, at the expense of some supporting code:
If you’re unable to change the signature of
someFunction
for whatever reason, using a temporary variable makes the code easier to read as well, sort of like simplifying conditionals by introducing a variable for no other reason than to make the code easier to parse by humans.2
Rename the variable so a bool value makes sense.
This a million times better than adding a comment to explain arguments to a function because the name is ambiguous.
2
Create a local boolean with a more descriptive name, and assign the value to it. That way the meaning will be more clear.
If you can’t rename the variable, then the comment should be a little more expressive:
2
There is a good article that mentions this exact situation as it refers to Qt-Style APIs. There, it’s called The Boolean Parameter Trap and it’s worth a read.
The gist of it is:
This is a bizarre comment.
From the compiler’s perspective,
someFunction(false /* forget */);
is actuallysomeFunction(false);
(the comment is stripped). So, all that line does is callsomeFunction
with the first (and only) argument set tofalse
./* forget */
is just the name of the parameter. It’s probably nothing more than a quick (and dirty) reminder, that doesn’t really need to be there. Just use a less ambiguous parameter name, and you won’t need the comment at all.One of advices of Clean code is to minimize number of unnecessary comments1 (because they tend to rot), and to name functions and methods properly.
Following that, I would just remove the comment. After all, modern IDEs (like eclipse) pop a box with code when you put a mouse over the function. Seeing the code should clear the ambiguity.
1 Commenting some complex code is ok.
2
To belabor the obvious, comments can lie. Thus is always better to make your code self-documenting without resorting to comments to explain, because some person (perhaps you) will change
true
tofalse
and not update the comment.If you can’t change the API, then I resort to 2 options
I like the answer about making the comment always true, but while good I think it misses the fundamental problem with this code — it is being called with a literal.
You should avoid using literals when calling methods. Local variables, optional parameters, named parameters, enums — how you best avoid them will depend upon the language and whats available, but do try to avoid them. Literals have values, but they don’t have meaning.
In C#, I used named parameters to make this clearer
Or
enum
:Or overloads:
Or polymorphism`:
The naming should always resolve ambiguity for booleans. I always name boolean something like ‘isThis’ or ‘shouldDoThat’, for example:
and so on. But when you are referencing someone else’s code, its best to just leave comment when passing values.
2
Filed under: softwareengineering - @ 09:26
Thẻ: boolean, coding-style, comments
« Could someone explain HaXe enums? ⇐ More Pages ⇒ Objects of different programming languages [closed] »