What's so bad about ref parameters?

Question

I'm faced with a situation that I think can only be solved by using a ref parameter. However, this will mean changing a method to always accept a ref parameter when I only need the functionality provided by a ref parameter 5% of the time.

This makes me think "whoa, crazy, must find another way". Am I being stupid? What sort of problems can be caused by a ref parameter?

Edit

Further details were requested, I don't think they are entirely relevant to what I was asking but here we go.

I'm wanting to either save a new instance (which will update with the ID which may later be used) or retrieve an existing instance that matches some logic and update that, save it then change the reference of the new instance to point to the existing one.

Code may make it clearer:

protected override void BeforeSave(Log entity)
{
    var newLog = entity;

    var existingLog = (from log in repository.All()
                           where log.Stuff == newLog.Stuff 
                                 && log.Id != newLog.Id
                           select log).SingleOrDefault();

    if (existingLog != null)
    {
        // update the time
        existingLog.SomeValue = entity.SomeValue;
        // remove the reference to the new entity
        entity = existingLog;
    }
}

// called from base class which usually does nothing before save
public void Save(TEntity entity)
{
    var report = validator.Validate(entity);

    if (report.ValidationPassed)
    {
        BeforeSave(entity);
        repository.Save(entity);
    }
    else
    {
        throw new ValidationException { Report = report };
    }
}

It's the fact that I would be adding it in only for one child (so far) of the base class that prevents me using an overload (due to the fact I would have to duplicate the Save method). I also have the problem whereby I need to force them to use the ref version in this instance otherwise things won't work as expected.

Answer 1

A ref parameter won't cause problems per se. It's a documented feature of the language.

However it could cause social problems. Specifically, clients of your API might not expect a ref parameter simply because it's rare. You might modify a reference that the client doesn't expect.

Of course you can argue that this is the client's fault for not reading your API spec, and that would be true. But sometimes it's best to reduce surprise. Writing good code isn't just about following the rules and documenting stuff, it's also about making something naturally obvious to a human user.

Answer 2

There is nothing wrong with using ref parameters that I can think of, in fact they can be very handy sometimes. I think they sometimes get a bad rap due to debugging since the value of the variable can change in the code logic and can sometimes be hard to track. It also makes things difficult when converting to things like WebServices where a "value" only pass will suffice.

Answer 3

Perhaps use an overloaded function for this 5% case and leave the other function as is.

Unnecessary ref parameters can lead to bad design patterns, but if you have a specific need, there's no problem with doing this.

Answer 4

Can you add an overload? Have one signature without the ref parameter, and one with it.

Ref parameters can be useful, and I'm glad they exist in C#, but they shouldn't be used without thought. Often if a method is effectively returning two values, it would be better either to split the method into two parts, or encapsulate both values in a single type. Neither of these covers every case though - there are definitely times when ref is the best option.

Answer 5

The biggest issue I have with ref parameters is they make it difficult to use type inference as you must sometimes explicitly declare the type of the variable being used as the ref parameter.

Most of the time I use a ref parameter it's in a TryGet scenario. In general I've stopped using ref's in that scenario and instead opted for using a more functional style method by way of an option.

For instance. TryGetValue in dictionary switches from

bool TryGetValue(TKey key, out TValue value)

To

Option<Value> TryGetValue(TKey key)

Option available here: http://blogs.msdn.com/jaredpar/archive/2008/10/08/functional-c-providing-an-option-part-2.aspx

Answer 6

If your method only needs this ref parameter 5% of the time perhaps you need to break this method down. Of course without more details its hard to say but this to me smells like a case of violating single responsability principal. Perhaps overloading it will help.

As for your question there is no issue in my opinion passing a parameter as a reference although it is not a common thing to run into.

Answer 7

If you take the .NET Framework as a barometer of people's expectations of an API, consider that almost all of the String methods return the modified value, but leave the passed argument unchanged. String.Trim(), for instance, returns the trimmed String - it doesn't trim the String that was passed in as an argument.

Now, obviously, this is only feasible if you're willing to put return-values into your API. Also, if your function already returns a value, you run into the nasty possibility of creating a custom structure that contains your original return value as well as the newly changed object.

Ultimately, it's up to you and how you document your API. I've found in my experience though that my fellow programmers tend to expect my functions to act "like the .NET Framework functions". :)

Answer 8

The most common use I've seen for ref parameters is as a way of returning multiple values. If that's the case, you should consider creating a class or struct that returns all the values as one object. If you still want to use a ref but want to make it optional, add a function overload.

Answer 9

This is one of those things that F# or other functional programming languages solve a lot better with returning Tuple values. Its a lot more cleaner and terse syntax. In the book I am reading on F#, it actually points out the C# equivelant of using ref as doing the same thing in C# to return multiple parameters.

I have no idea if it is a bad practice or there is some underlying "booga booga" about ref parameters, to me they just feel as not clean syntax.

Answer 10

A void-returning function with a single reference parameter certainly looks funny to me. If I were reviewing this code, I'd suggest refactoring the BeforeSave() to include the call to Repository.Save() - renaming it, obviously. Why not just have one method that takes your possibly-new entity and guarantees that everything is saved properly? The caller doesn't do anything with the returned entity anyway.

Answer 11

An overload won't kill your application or its design. As long as the intent is clearly documented, it should be okay.

One thing that might be considered is mitigating your fears about the ref parameter through a different type of parameter. For example, consider this:

public class SaveArgs
{
   public SaveArgs(TEntity value) { this.Value = value; }

   public TEntity Value { get; private set;}
   public int NewId { get; internal set; }
   public bool NewIdGenerated { get; internal set; } 
}

In your code, you simply pass a SaveArgs rather than the TEntity, so that you can modify its properties with more meaningful information. (Naturally, it'd be a better-designed class than what I have above.) But then you wouldn't have to worry about vague method interfaces, and you could return as much data as you needed to in a verbose class.

Just a thought.

EDIT: Fixed the code. My bad.