Should you change return types in a method overload?

The design guidelines around method overloading are clear… you should only overload methods that do semantically the same thing.  That is you should be able to change your code from calling one to calling another without substantive changes to the rest of your code.

Someone recently forwarded me this thread post on change the return type in an overload.  I completely agree with the conclusion reached here… I don’t see how you can change the return type and keep the method semantically the same.

 

As someone in the thread commented, simply choose a more appropriate name for the new method.  

 

Wrong:  

Public Function MyFunc(someParam as String) as MyCustomClass

Public Function MyFunc(someDifferentParam as Type, someParam as

String) as MyCustomClass

Public Function MyFunc(someDifferentParam as Integer) as Boolean

 

Right:

Public Function MyFunc(someParam as String) as MyCustomClass

Public Function MyFunc(someDifferentParam as Type, someParam as

String) as MyCustomClass

Public Function IsMyFunc(someDifferentParam as Integer) as Boolean

 

Based on the discussion, we should likely add a guideline to spell this out clearly:

 - Do not change the return type when overloading a method

 

 

Thoughts, comments?

 

Published 16 July 05 11:41 by BradA

Comments

# Douglas McClean said on July 17, 2005 3:48 AM:
I think changing the return type when overloading can make sense. Most commonly I would expect to see this when the return type of each method in the method group is the same as one the type of a parameter:

public float Sin(float radians);
public double Sin(double radians);

Many of the cases for this may be solved by generics though:

public int Min(int a, int b);
public double Min(double a, double b);

.. becomes ..

public T Min<T>(T a, T b) where T : IComparable<T>;
# Matthijs van der Vleuten said on July 17, 2005 4:05 AM:
Isn't that the same as this?

public IComparable Min(IComparable a, IComparable b);
# Tobin Titus said on July 17, 2005 4:59 AM:
I like the idea of having a design guideline for this. It seems a bit silly to modify an overload to the point that it doesn't perform the same function. If its returning a different data type, it isn't performing the same function as its overloaded counterpart.
# geoff.appleby said on July 17, 2005 5:08 AM:
Hey Brad,

While I was already sure I was correct, and then validated by the fact that the rest of the codebetter gyus also agreed, it's nice to have it from a 'softie :)

One that none of us really touched was having some with no return type at all. For example:

Public Sub Add(X as SomeItem)
Public function Add(X as String) as SomeItem

This is an obvious one in that it seems silly to return the same object as was passed in in the first place. But at the same time...is it? I think it's nice to still retain the consistancy...but perhaps then it could lead to confusement that perhaps you were getting back something different than the one you passed in.

It's definitely worth getting a guideline out there for it. I scoured around and found nothing - now people can find my post (and yours now too) - but an official guideline (that can always be ignored :) is more likely to sway peoples actions.

--Geoff
# TAG said on July 17, 2005 8:51 AM:
Is you return different types it will be good to make it safe for consumers by adding some modifier to function name to make it clear that type is returned.

In the same time - if you will take a look on Activator.CreateInstance methods family ( http://msdn.microsoft.com/library/en-us/cpref/html/frlrfsystemactivatorclasscreateinstancetopic.asp ) - you will see that return types ARE different and very dangerous. One set of methods return object, while another ObjectHandle.
If for some reasons somebody will change argument types - then compiler can be unable to detect change of return type from object to ObjectHandle.
This is clear that instead of CreateInstance there must be used CreateObjectHandle method name.
# Chris said on July 17, 2005 10:08 AM:
As a rule I agree that you shouldn't change the return value of an overloaded function. But as Microsoft has already proven, there are exceptions. Take the typed dataset; when you add rows you can use the function that takes all the column values as input and return a reference to the new row, or you can use the function that takes a reference to an already existing row and doesn't return anything.

This type of scenario is probably relevant for other people designing class libraries, so you might want to include an exception that handles these types of cases.
# Josh Einstein said on July 17, 2005 12:22 PM:
I think this is the first time I am posting but I am a long time reader and a big fan of Brad Abrams. Watched all the web casts and I admire his stance on consistency and predictability in the framework.

However, I have to disagree that it is *always* wrong to change the return type. For example, as others have stated, some math oriented functions do semantically the same thing but on different types.

I'd hate to see:

int RoundInt32(int number);
decimal RoundDecimal(decimal number);
float RoundSingle(float number);

I'd prefer to see a single overloaded Round method.

int Round(int number);
decimal Round(decimal number);
float Round(float number);

As someone else suggested, generics could potentially get around this but maybe not. Especially if there are widely different code paths within.
# Keith Patrick said on July 17, 2005 4:42 PM:
Few thoughts:
Overloads should be used to create default params. A) it's confusing what the default param value would be, and it makes the API complex while adding confusion, B) the overloaded method should do exactly the same thing as its siblings with different symantical parameters, so no signature changes, C) MS shouldn't use VB for its .Net sample code as C# has a greater similarity footprint (C#, C++, Java, Pascal); I think it's harder to convert VB into C# than vice versa.
# TrackBack said on July 17, 2005 9:29 PM:
# TrackBack said on July 17, 2005 9:41 PM:
# TrackBack said on July 17, 2005 10:21 PM:
# Bill's random thoughts.... said on July 17, 2005 10:23 PM:
# TrackBack said on July 17, 2005 10:29 PM:
# TrackBack said on July 17, 2005 10:31 PM:
# mihailik said on July 18, 2005 6:57 AM:
Brad, you should not add this rule to your Design Guidelines. There are cases where changing return type is right thing.

Most common case is typed collections. When you override indexer you change it's return type.

As well it can be applied to Clone() or Copy() methods.

It means "specializing" return type is needed sometimes when you are specializing "generic" pattern to be more typed.
# geoff.appleby said on July 18, 2005 7:02 AM:
mihailik: the thing here is the difference between overloading and overriding.
# Kevin Westhead said on July 18, 2005 8:11 AM:
Matthijs, the difference between

public T Min<T>(T a, T b) where T : IComparable<T>;

and

public IComparable Min(IComparable a, IComparable b);

is that the generic version avoids any boxing, while still requesting IComparable behaviour.
# Darren Oakey said on July 18, 2005 9:49 AM:
I would say that there are exactly two times when it is ok to overload functions

a) when you use overloads for creating essentially default parameters. In this case, you are talking about a specific bit of functionality - and it would be very wrong, and very confusing, to have two different return types - imagine
PrintStatus PrintReport( blah )
and
PrintResult PrintReport( blah )
that's just a mistake waiting to happen - there is no obvious direction for the user

b) in the world before generics, overloads were used to create fairly generic functions
for example:
class SqlTypes
static string Safe( SqlString value );
static int Safe( SqlInt32 value );

etc - and in this case, you almost ALWAYS want to change the return type.

However, now that we have templates - almost every time you would want to do this, you could do it better by making a template

So - in VS-2003 YES - change return types for pseudo-generic functions only

in VS-2005 NO - don't change return types :)
# Grim said on July 18, 2005 11:22 AM:
I don't think you should add this. There are way too many people (i.e. think pointy-haired bosses) that think "Microsoft guidelines" and "Word of God" are synonymous.

While in most cases is is a reasonable guideline, there are cases where it is appropriate, or at least not inappropriate, to have different return types from overloads. It depends largely on the purpose of the method and the reason for the overload.

For instance, an XmlUtility static class with these methods:

public static void XslTransform(string sourceFile, string styleSheet, string outFile) {}

public static Stream XslTransform(Stream sourceXml, Stream styleSheetXml) {}

public static XmlDocument XslTransform(XmlDocument sourceDocument, XmlDocument styleSheetDocument) {}
# Sahil Malik said on July 18, 2005 1:36 PM:
I agree, don't do it !!
# Andy Alm said on July 19, 2005 3:03 PM:
I might add to Grim's comments that the XslTransform class itself actually uses a different return type on different overloads of the XslTransform.Transform method. An example:

public XmlReader Transform(IXPathNavigable input, XsltArgumentList args, XmlResolver resolver);

and

public void Transform(XPathNavigator input, XsltArgumentList args, XmlWriter output, XmlResolver resolver);

I agree that you should typically avoid having different return types on an overloaded method, but there are always exceptions to just about any rule right?

# Joshua Bair said on July 27, 2005 1:57 AM:
There are also different return types from the Add function in the TreeView and TreeNode classes. In one case, you are calling the Add function and requesting that the function create a new node, and the new node will be passed back. In the other case, you have already created the TreeNode object, and you are requesting that the function add YOUR TreeNode to the TreeView.

I would agree that on the surface, saying "don't change return types" makes sense, but in practice, there are times when it makes sense to return what the user is asking for.

- Joshua
# Mike Tours said on August 25, 2005 10:59 AM:
Similarly to the above comment about the “Add” function, I have “Create” function which adds a record to a database. Sometimes it makes sense to return the entire row of the new record (for display purposes), and other times it only makes sense to return just the identity value of the new record ( like when I need to create another record with the previous record as a foreign key). It still stands that the functionality for the Create method remains the same. Returning something is not what it’s specifically designed to do. Returning the data as part of the initial call just saves one extra trip back to the DB. I don’t like the idea of having different names for the overloads : “CreateAsDataset” and “CreateAsLong“.

The other option we discussed was having the same function name, but having the return types as out parameters. This means both overloads will be void, and overload as per the guidelines, but no one would agree on this format, just because they didn’t like it not because it’s wrong.

So yeah, in general I agree it that the guidelines should be adhered to, but in practice it doesn’t always make perfect sense.
# Jeff Becker said on September 19, 2005 1:10 PM:
I agree with the sentiment in general. There are simply times when I want the base from which im inheriting to be General, and the Derived class to be spesific. The best example of this i can think of is my DataAccess Layer.

I Generally define a class DataLayerBase which encapsulates all sorts of stuff and exposes at least 3 methods:
Object Get(int);
void Save(Object);
void Delete(Object);

Then i go on to define DataLayer classes for my Object Model. The Link Class would get a Corresponding LinkDataLayer which handles the nitty-gritty of actually moving data to and from the database. In LinkDataLayer, I would override the Get function to return an instance of the Link class. Yes, I could just cast the Object into a Link but symantically it 'feels better' this way.
New Comments to this post are disabled

Search

Go

This Blog

本博客中文版 (View in Chinese)



These postings are provided "AS IS" with no warranties, and confer no rights.

Syndication

Page view tracker