Intentionally incorrect use of language features, specifically "ref" in C#, as a hint to colleagues

Question

I recently came across a piece of code something like this (roughly C#):

public bool ValidateStuff(ref ArrayList listOfErrors, Stuff thingsToValidate)
{
    if (!thingsToValidate.isValid() )
    {
        errors.add("New error!");
    }
}

ArrayList errors = [];
bool valid = ValidateStuff(ref errors, stuffToValidate);

The key thing is the "ref" keyword - there is no need to use that keyword given the functionality here, and when I asked why it was there I was told that it had only been added as a kind of warning that the errors parameter is manipulated by the method, it's not purely used as input.

This immediately seemed wrong to me (ref has a purpose, and here it is not being used according to its purpose), but the idea of forcing (since you have to use ref when calling the method) a reminder of slightly counter-intuitive usage seemed reasonable, and I couldn't immediately think of a better way, apart from liberal use of comments. What do you think?

Extra credit: how could this method be better? Perhaps by simply returning a new List of errors which is added to the main List in the calling code? By providing two methods, one for the test and one for the errors? Or is it fine as it is?

Answer 1

Understand that the ref keyword signifies an intent to reassign the pointer.

If you enable Microsoft's code analysis, you will get a warning telling you that using ref param is likely a bad design.

Passing by ref is not just a 'visual' or syntactic sugar change. It actually allows the method to reassign the reference to a new object, which affects the calling code in ways that did not intend.

For example, without using ref, if the method tries to re-assign the paramter variable to a new object, it will NOT have any impact on the calling code's copy of the variable.

public bool ValidateStuff(ArrayList listOfErrors, Stuff thingsToValidate)
{
    if (!thingsToValidate.isValid() )
    {
        listOfErrors = new ArrayList(){"error"};
    }
}

ArrayList errors = ["existing error1", "existing error2"];
bool valid = ValidateStuff(errors, stuffToValidate);
Console.WriteLine(errors);  //output is still "existing error1", "existing error2"

But the same thing using ref, actually changes the reference outside of the method:

public bool ValidateStuff(ref ArrayList listOfErrors, Stuff thingsToValidate)
{
    if (!thingsToValidate.isValid() )
    {
        listOfErrors = new ArrayList(){"error"};
    }
}

ArrayList errors = ["existing error1", "existing error2"];
bool valid = ValidateStuff(ref errors, stuffToValidate);
Console.WriteLine(errors);  //output is now "error"

Now, in both of these pseudo code examples demonstrate a bug in the code. The important difference is that, the first version is likely to cause a predictable error. The second example could cause very unpredictable results in certain situations.

What to do instead

Now that you've (hopefully) accepted the fact that the language designers intended for ref to only be used when the method intends to reassign the pointer, you have only a few options:

1. Redesign your method to return the error list instead of being a void.
2. Add a new class containing everything that needs to be returned, and return a new instance of that class.
3. Pass multiple reference-type parameters (without using the ref keyword) and let the method modify them as necessary.

Beyond that, there's nothing else you can do. The language was designed to be this way. Don't drive yourself crazy over the fact that C# doesn't allow multiple return values. I heard Go supports multiple return values if you can't accept it :D

Answer 2

Ugh. Always impressive when humanity shows how creatively terrible it can be...

Yes, the proper approach here is to return an IEnumerable of whatever your error type is (unless you're just using exceptions, in which case AggregateException exists). If you're doing anything remotely fancy with the errors, a special object that has a pass/fail property and is or exposes an IEnumerable of error type is the next step.

Answer 3

In the early days of .NET and my endeavors into .NET land I used the same mechanism, i.e. to signal callers that the collection / container object they passed to the method was changed inside the method. But after a while I realized what is better worded in Dan Ling's answer above, namely that this is actually wrong: ref has a different meaning, and it actually illustrates something else, namely that the collection passed in can be a completely different object when the method returns.

It's better to document using XML document tags (that's what they're for!) what the behavior is of the method than signaling it through usage of 'ref'.