Matias Cicero
Matias Cicero

Reputation: 26281

Difference between returning reference vs not returning anything

Is there a difference between these two methods?

public class A
{
    public int Count { get; set; }
}

public A Increment(A instance)
{
    instance.Count++;
    return instance;
}

public void Increment(A instance)
{
    instance.Count++;
}

I mean, apart from one method returning the same reference and the other method not returning anything, both of them accomplish the same thing, to increment the Count property of the reference being passed as argument.

Is there an advantage of using one against the other? I generally tend to use the former because of method chaining, but is there a performance tradeoff?

One of the advantages of the latter method, for example, is that one cannot create a new reference:

public void Increment(A instance)
{
    instance.Count++;
    instance = new A(); //This new object has local scope, the original reference is not modified
}

This could be considered a defensive approach against new implementations of an interface.

I don't want this to be opinion based, so I am explicitly looking for concrete advantages (or disadvantages), taken out from the documentation or the language's specification.

Upvotes: 1

Views: 99

Answers (2)

Jon Hanna
Jon Hanna

Reputation: 113272

One of the advantages of the latter method, for example, is that one cannot create a new reference.

You could consider that one of the disadvantages. Consider:

public A Increment(A instance)
{
  return new A { Count = instance.Count +1 };
}

Or

public A Increment()
{
  return new A { Count = this.Count +1 };
}

Apply this consistently, and you can have your A classes being immutable, with all the advantages that brings.

It also allows for different types that implement the same interface to be returned. This is how Linq works:

Enumerable.Range(0, 1)    // RangeIterator
  .Where(i => i % 2 == 0) // WhereEnumerableIterator<int>
  .Select(i => i.ToString()) // WhereSelectEnumerableIterator<int, string>
  .Where(i => i.Length != 1) // WhereEnumerableIterator<string>
  .ToList();                 // List<string>

While each operation acts on the type IEnumerable<int> each result is implemented by a different type.

Mutating fluent methods, like you suggest, are pretty rare in C#. They are more common in languages without the sort of properties C# supports, as it's then convenient to do:

someObject.setHeight(23).setWidth(143).setDepth(10);

But in C# such setXXX methods are rare, with property setters being more common, and they can't be fluent.

The main exception is StringBuilder because its very nature means that repeatedly calling Append() and/or Insert() on it with different values is very common, and the fluent style lends itself well to that.

Otherwise the fact that mutating fluent methods aren't common means that all you really get by supplying one is the minute extra cost of returning the field. It is minute, but it's not gaining anything when used with the more idiomatic C# style that is going to ignore it.

To have an external method that both mutated and also returned the mutated object would be unusual, and that could lead someone to assume that you didn't mutate the object, since you were returning the result.

E.g upon seeing:

public static IList<T> SortedList(IList<T> list);

Someone using the code might assume that after the call list was left alone, rather than sorted in place, and also that the two would be different and could be mutated separately.

For that reason alone it would be a good idea to either return a new object, or to return void to make the mutating nature more obvious.

We could though have short-cuts when returning a new object:

public static T[] SortedArray<T>(T[] array)
{
  if (array.Length == 0) return array;
  T[] newArray = new T[array.Length];
  Array.Copy(array, newArray, array.Length);
  Array.Sort(newArray);
  return newArray;
}

Here we take advantage of the fact that since empty arrays are essentially immutable (they have no elements to mutate, and they can't be added to) for most uses returning the same array is the same as returning a new array. (Compare with how string implements ICloneable.Clone() by returning this). As well as reducing the amount of work done, we reduce the number of allocations, and hence the amount of GC pressure. Even here though we need to be careful (someone keying a collection on object identity will be stymied by this), but it can be useful in many cases.

Upvotes: 1

trailmax
trailmax

Reputation: 35106

Short answer - it depends.

Long answer - I would consider returning the instance of the object if you are using a builder pattern or where you need chaining of methods.

Most of other cases do look like a code smell: if you are in control of the API and you find a lot of places where your returned object is not used, so why bother with extra effort? possibly you'll create subtle bugs.

Upvotes: 1

Related Questions