views:

139

answers:

8

I've been bitten a couple of times by statements in VB.NET (not sure if this effect exists in C#) that appear to be self-referencing, but when they're executed, they don't actually do anything because they require a target and one isn't provided. For example:

Dim MyString as string = "String to test"

' Neither of these lines do anything '
MyString.Replace(" ", "-")
MyString.Substring(0,5)

' This will return the original string, because neither statement did anything '
Messagebox.Show(MyString)

In both cases, it doesn't seem to bother .NET that the statement needs a target to assign the result to, and I don't give it one. Is there a reason that the IDE/compiler doesn't warn me of this effect, or that an exception "StatementDoesntDoAnything" isn't thrown? Since the code is formed in such a way that it will never change anything, it's clearly mis-typed.

+2  A: 

There are many instances where methods return values that are optionally handled by the programmer. There is no way for the compiler to know that this particular method does nothing. It might have a side effect and the fact that you choose to do nothing with the return value of the method has to be a concious decision on your part.

If the compiler choose to warn you of every instance that this happened then you will get too many false warnings.

Vincent Ramdhanie
This is correctly, though theoretically they could add a warning with a particular warning level, and you could turn off warnings with that level.
Nick
I'd considered this but never tried it - I just created a custom function as did nothing but returned a value, then called it and didn't handle the return, with the same effect. It seems like I should be able to decorate my functions with a <ReturnHandlerRequired> or something like that to specify that they have no impact if the return is discarded, to prevent situations like this from happening. The case with VB.NET's string functions is a perfect example - they have no side-effects, and so calling them without handling the results is pointless, so should be flagged.
rwmnau
I have to respectfully disagree. Languages like OCaml and F# will actually warn you for not using a (non-void) value, and I find it to be a very helpful debugging tool. If you need to call a function without using its return value, you write "let _ = someFunc()" or "someFunc() |> ignore" to swallow the value without using it.
Juliet
Juliet - I like this convention more, because I have to signify that I'm swallowing the return value, as opposed to accidentally doing it. My problem is not so much with functions that don't "do" anything, but with functions I'd expect to be reflexive - like string functions, which are clearly meant to act on the string itself - though I suppose that's harder to qualify objectively...
rwmnau
+4  A: 

Yes, it would be great if methods which have no side-effects could be marked with some kind of [NoSideEffectsAttribute()] so that tools like compilers can warn you, but at present no such thing is known to me.

But you could try FxCop, it can spot lots of subtle programming errors on .NET assemblies.

codymanix
Prime task for a static analysis tool.
Malaxeur
The .net 4 code contracts library contains a Pure attribute which does this.
Lee
+3  A: 

It can be hard to tell that ignoring the return value isn't intended, as some functions do some side-effect and return a value.

Functions which "merely" return values would have to be marked as such to have the compiler check them, and that just hasn't been a priority or judged to have enough return on investment (obviously, otherwise they'd have done it :).

Roger Pate
A: 

This is inherited behavior from C, C++, and was orioginally done so that you can choose whether or not to use the return value from a function/method... When you write a function/method that does a bunch of stuff and then returns some value, when you call it, you have the option of writing

variableName = functionName([parameterlist]);

if you want to use the returnb value in something, or just

functionName([parameterlist]);

if you don't.

For function methods that have no side effects (like the ones you mention) as you noted, this doesn't quite make sense, but changing it for new languages would run counter to the long history of many many other languages that conform to this standard...

Charles Bretana
A: 

I'm not sure having yet another keyword for forcing or not the return value to be put into a variable would be a good idea for various reason

1 ) Suppose you add a keyword when the return value is optinal (hence implying it is usually mandatory) would cause lots of code to stop compiling or issued tons of warning

2) Suppose you do the reverse, adding a keyword when the return value is forced, some people would simply use dummy variables to store them and keep using it the way ther used to.

3) I don't think lots of people would actually take the time to ponder whether or not the return value is optional or not. In some case, I got to admit it's given, but not always.

David Brunelle
A: 

I've been bitten a couple of times by

jumping to conclusions, not by

statements in VB.NET ... that ... don't actually do anything

but that are actually very well documented.

Justice
Perhaps I should rephrase my question. It's not about the documentation - which is correct - but about accidentally forming it incorrectly, but in a way that runs without any sign that something's wrong. I don't expect my code to debug itself, but this mistake seems obvious and easy for the compiler to catch.
rwmnau
*Strings are immutable.* All methods that appear to mutate strings actually return new strings and do not mutate the original strings.
Justice
**Especially** given that strings are immutable, it's ridiculous that the compiler doesn't catch this.
rwmnau
*All* methods are considered to have side effects, unless proven otherwise. There is no general way for the compiler to "prove otherwise." The only way is for the BCL team to go through by hand and mark all "pure" functions with a `[PureAttribute]` attribute, something they are actually doing with the CodeContracts framework.
Justice
If you want a language with a compiler that can catch all your no-ops, consider using Haskell.
Justice
+1  A: 

It is often difficult for the compiler to determine if the functions "do something". Unless the compiler does rigorous Inter-Procedural Analysis (IPA), it can not determine if the function call has a side effect.

IPA is a slow process that significantly increases the compilers memory requirements so by default most compilers do not perform this.

scurial
A: 

Consider the method Dictionary<TKey, TValue>.TryGetValue(TKey key, out TValue value): it checks if the key is in the dictionary, and if it is, it puts the value into the out parameter. The return value is a bool indicating whether the operation was successful. Sometimes you care; sometimes you don't. I think this is a pretty accepted approach for methods like this; if the compiler forced you to assign the return value to a variable, people would have a lot of code like this:

int someValue = 0;
bool discard = IntDictionary.TryGetValue("key", out someValue);

// I don't even care if discard is true or false;
// someValue will be 0 if it wasn't in IntDictionary
Dan Tao
This is true - there are a number of instances where a developer could choose to ignore the return value, and I'd hate to add throwaway code everywhere, since it makes things more difficult to read.
rwmnau