Defaulting to passing reference types using the ref keyword

Question

I work with a developer who defaults to passing reference types such as StringBuilder, string and MemoryStream using the ref keyword. They do this regardless of whether they need to actually change the reference itself.

public void ExampleMethod(ref MemoryStream ms)
{
    byte b=ms.ReadByte();
    ...
    // No changing of actual ms reference such as: ms=new MemoryStream();
}

Almost always, the methods just use the object and return without changing the reference in any way. For the immutable types ie string, this is sometimes necessary, but why for the mutable types?

To me this has a bit of a "code smell" in that it could lead to less maintainable code in that it is more permissive than what it really needs to be.

However is this something that is serious enough for me to bring up with the developer? My intial feeling is yes, but maybe this is too pedantic?

Answer 1

Perhaps you could ask that developer why he does that ... Maybe he (wrongly) thinks it is more performant ? Maybe that developer is a C++ developer, who thinks that using ref is similar as using a pointer in C++ ?

If he comes from a C++ background, I would ask the developer why he does that, and explain to him that this is totally not necessary. That it does not improve performance, and that it instead gives different semantics to the method, since it allows to change the reference, and that you should only use the ref keyword when it is indeed necessary that the method can change the reference.

I don't think it is pedantic. Even an experienced developer doesn't know all the in and outs of a new language he's using. When you explain it to him, he will learn something new, and maybe he'll be gratefull. :)

Answer 2

The ref keyword is only useful for when modifying value types (anything that descends from ValueType). Value types are things such as int, byte, char, struct, float etc. Using ref for object references or classes such as Stream descendants or StringBuilder does not make sense and is definitely a code smell.

The only time I've seen it necessary to pass a StringBuilder reference as ref is when marshalling string values to and from Win32.

Answer 3

I don't think it is too pedantic because the developer may have misunderstood what the "ref" keyword is good for. It should be used where absolutely necessary, indicating that the method will guaranteed or most likely swap your reference with something else - not "just for fun".

Update
As to how to communicate it...hm, depends maybe somewhat on the motivation behind why he's putting ref on all things. If he thinks that it will improve performance then do a little program using StopWatch.StartNew() and passing things into a method with and without ref. I haven't tried it but would think that the performance difference may lie in creating a new reference and that should be damn small.

Apart from that features should be used as they were intended and to express intent. When I see a method with ref or out parameters I expect them to either completely change the instance passed in or that the method initializes the passed in object(s), respectively. Not using those keywords for those purposes is simply a way to confuse fellow programmers, by cluttering method calls with unnecessary keywords and possibly setting up wrong expectations regarding the API used.

Answer 4

It looks like a misunderstanding; that the ref keyword would be needed to pass references to a method. You should ask the developer if that is the case.

The only good reason to use the ref keyword when it's not needed, is to allow the method to be changed later on to substitute the object. This should of couse only be used where you really anticipate that you will need it. Normally you should just code for the current requirements, not every possible requirement that you can imagine.

If you find a ref that is not needed for any reason, it should be removed from the code. Following the principle of encapsulation, the method should not be given more power over the parameter values than it needs.

Generally, the ref keyword should rarely ever be used. In most cases a more object oriented approach can be used instead. It's used sometimes with value types for performance reasons, like in the Double.TryParse method, but never really with a reference type.

Answer 5

It's definitely worth bringing it up. I've seen this a number of times and it's always been due to developers not understanding the type system - which is a crucial part of writing correct and maintainable code.

I would find an example where it's obviously not necessary, but where they want to change the contents of the object itself - something that appends to a StringBuilder would be ideal. Then just ask, politely, why they've chosen to use the ref modifier. Point out that it will work just as well without.

Feel free to refer them to my parameters article and my reference/value type article.