views:

44

answers:

4

Disclaimer

Please don't just vote to close this because the title looks subjective, and if it's been asked before please point me to the previous question in the comments and I'll delete this one -- I really did look high and low trying to find a previous question on the subject.

Background

Given I have the following interface and concrete implementation:

public interface IFoo 
{
   // Some stuff
}
public class Foo : IFoo
{
   // Concrete implementations of stuff
}

And somewhere I have the following method:

public Foo GiveMeAFoo()
{ 
    return new Foo();
}

I have traditionally always returned Foo, seeing as it is inherently an IFoo anyway, so it can be consumed at the other end as an IFoo:

IFoo foo = GiveMeAFoo();

The main advantage of this that I can see is that if I really need to consume Foo somewhere as the concrete implementation, I can.

Actual Question

I recently came across some comments on another question, giving someone a hard time for doing this, suggesting the return type should be IFoo.

Am I wrong or are they? Can anyone give me a reason why returning the concrete implementation is a bad idea?

I know that it makes sense to require an IFoo when receiving a parameter to a method, because the less specific a parameter is, the more useful a method is, but surely making the return type specific is unreasonably restrictive.

Edit

The use of IFoo and Foo might have been too vague. Here's a specific example from .NET (in .NET, arrays implement IEnumerable -- i.e. string[] : IEnumerable<string>)

public string[] GetMeSomeStrings()
{
    return new string[] { "first", "second", "third" };
}

Surely it's a bad idea to return IEnumerable here? To get the length property you'd now have to call Enumerable.Count(). I'm not sure about how much is optimised in the background here, but logic would suggest that it's now going to have to count the items by enumerating them, which has got to be bad for performance. If I just return the array as an array, then it's a straight property lookup.

+3  A: 

If you want your method to be the most flexible that it can be, you should return the least derived type (in your case, IFoo):

public interface IFoo { }

public class Foo : IFoo { }

public IFoo GiveMeAFoo() { return new Foo(); }

That will allow you to change the concrete implementation of IFoo internal to your method without breaking anybody that is consuming your method:

public interface IFoo { }

public class Foo : IFoo { }

public class Foo2 : IFoo { }

public IFoo GiveMeAFoo() { return new Foo2(); }
Justin Niessner
But anyone using GiveMeAFoo can always treat it as an IFoo if they want to, and then their code wouldn't break.
Bennor McCarthy
Yes, but you're relying on them to do the right thing. In the case of returning IFoo, you're telling the consumer that they should treat the return value as the interface type and not the concrete type...that way they aren't relying on anything only implemented by Foo.
Justin Niessner
+1  A: 

You can create a bunch of interfaces and give it to different teams. Taking your own example

public interface IFoo 
{
   // Some stuff
}

public interface IBar
{
    public IFoo getMeAFoo();
}

Then you can give these interfaces to a person developing a front end app and he doesn't have to know what the concrete implementation is. The guy developing the front end can use the getMeAFoo() method knowing that it returns an object of IFoo whereas the concrete implementation can be developed separately as:

public class Foo implements IFoo
{
    // more stuff
}

public class MoreFoo implements IFoo
{
    //
}

public class WunderBar implements IBar
{
    public IFoo getMeAFoo()
    {
        case 1:
            return new Foo();
        case 2:
            return new MoreFoo();
    }
}

Hope this makes sense :)

Sagar V
A: 

Fine, everywhere you use it, you use IFoo foo = GiveMeAFoo();

Then someone else uses your code and doesn't use it that way, for whatever reason, they use Foo foo = GiveMeAFoo();

Now they see all the functionality of Foo that isn't (for good reason) part of IFoo. Now they can use parts of the implementation that aren't part of the interface. Now you can't change your implementation without breaking their code, your implementation is now the public API and you are going to tick people off if you try to change your implementation details that they are counting on.

Not good.

mattjames
+1  A: 

In my opinion, it's always better to return the most abstract type you can. If you find yourself in need for anything that's specific to the type you ar actually returning, I would consider it a code smell.

The main advantage of this is that you can change your mind about which type you really want to return. The called method is made more decoupled from its callers.

Also, you can possibly remove dependencies. The caller code don't need to know anything about the actual type you chose to create.

My final remark is a quotation from Eric Lippert: "You probably should not return an array as the value of a public method or property". (His article is focused in C#, but the general idea is language-agnostic)

jpbochi