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?


 

Comments (24)

  1. Douglas McClean says:

    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>;

  2. Isn’t that the same as this?

    public IComparable Min(IComparable a, IComparable b);

  3. Tobin Titus says:

    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.

  4. geoff.appleby says:

    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

  5. TAG says:

    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.

  6. Chris says:

    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.

  7. 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.

  8. Keith Patrick says:

    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.

  9. mihailik says:

    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.

  10. geoff.appleby says:

    mihailik: the thing here is the difference between overloading and overriding.

  11. Kevin Westhead says:

    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.

  12. Darren Oakey says:

    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 🙂

  13. Grim says:

    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) {}

  14. Sahil Malik says:

    I agree, don’t do it !!

  15. Andy Alm says:

    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?

  16. Joshua Bair says:

    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

  17. Mike Tours says:

    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.

  18. Jeff Becker says:

    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.

Skip to main content