It is necessarily bad style to ignore the return value of a method
Let's say I have a C # method
public void CheckXYZ(int xyz) {
// do some operation with side effects
}
public int GetCheckedXYZ(int xyz) {
int abc;
// functionally equivalent operation to CheckXYZ,
// with additional side effect of assigning a value to abc
return abc; // this value is calculated during the check above
}
CheckXYZ
and replacing all existing calls
CheckXYZ()
with
GetCheckedXYZ()
, ignoring the return value? The return type is not
IDisposable
in this case. Is it up to discretion?
EDIT: After all the answers, I expanded on this example a bit. (Yes, I understand what he got out
in it now, especially for @Steven)
public void EnsureXYZ(int xyz) {
if (!cache.ContainsKey(xyz))
cache.Add(xyz, random.Next());
}
public int AlwaysGetXYZ(int xyz) {
int abc;
if (!cache.TryGetValue(xyz, out abc))
{
abc = random.Next();
cache.Add(xyz, abc);
}
return abc;
}
a source to share
It totally depends on what the return value tells you and if it's important to know or not. If the data returned by the method is irrelevant to the code that calls it, then the ignore completely is valid. But if it indicates some kind of rejection / counter / influencer value, then ignore it at your own risk.
a source to share
This is usually bad style, yes. This is allowed and ok when methods return a class instance for chaining (foo.bar (). Baz (). Xyz (). Asdf () => asdf returns an instance of foo, but you don't need it anymore)
In your case, the point of bad style is not an ignored return value, but methods with side effects. The CheckXyz () function must always return a boolean value and has no additional side effects.
In general, the side effects are bad, and if you call a method and can ignore the return value, it means that the method / object / library / program might be poorly designed.
a source to share
IMHO, it is generally best to have one (and only one) way of doing things to avoid duplicating your codebase. Usually, if you use and sometimes don't use the return value, this is probably a sign that your code could be broken better. In your example, it would probably be nice if both of these functions called the third (common) function to avoid duplication of core functionality.
Error codes should always be checked and processed, but if a function simply returns information, then what you do with that information is up to you.
[Edit] ... and as dbemerlin points out, side effects should be avoided whenever possible.
a source to share
The general convention of C is as follows:
(void)GetCheckedXYZ();
Casting to void has no effect, but by convention it shows that the developer knows the return value is being ignored, i.e. shows it on purpose.
C # won't let you do this, but I saw this instead (also in Java):
/*(void)*/GetCheckedXYZ();
Some may lack aesthetics, but they convey the intent of the developer without resorting to alternative methods, which I think are worse.
a source to share
Many of these answers have good points. I'll just add that if you choose to ignore the return value, comment it out on the lines "don't care about the return value because ..." so that the next person going into the code will see that you didn't miss it by accident and that you thought everything through
EDIT: Better, put your comment in an empty block
if (!something()) {
// Not worried if this fails because blah
}
a source to share