Is it advisable to have an interface as the return type?

Question

I have a set of classes with the same functions but with different logic. However, each class function can return a number of objects. It is safe to set the return type as the interface?

Each class (all using the same interface) is doing this with different business logic.

protected IMessage validateReturnType; <-- This is in an abstract class

public bool IsValid() <-- This is in an abstract class
{
    return (validateReturnType.GetType() == typeof(Success));
}

public IMessage Validate()
{
    if (name.Length < 5)
    {
        validateReturnType = new Error("Name must be 5 characters or greater.");
    }
    else
    {
        validateReturnType = new Success("Name is valid.");
    }

    return validateReturnType;
}

Are there any pitfalls with unit testing the return type of an function? Also, is it considered bad design to have functions needing to be run in order for them to succeed? In this example, Validate() would have to be run before IsValid() or else IsValid() would always return false.

Thank you.

Answer 1

However, each class function can return a number of objects. It is safe to set the return type as the interface?

This is good practice and common. For example look at the way COM was built, it heavily relies on this methodology.

Are there any pitfalls with unit testing the return type of an function

No.

Also, is it considered bad design to have functions needing to be run in order for them to succeed? In this example, Validate() would have to be run before IsValid() or else IsValid() would always return false.

That's fine for working with an object oriented programming paradigm, take for example working with sockets. It's common to have a connect method before you can send and receive data.

That being said it's good to keep less state than more state as a general rule because in that way it's easier to prove your program is correct. For example you have to test each function that relies on this function not in one way but in 2 ways now. Possible program states grows exponentially if you have a lot of state. Take a look at functional programming if you are interested in why state is a bad thing.

Answer 2

Also, is it considered bad design to have functions needing to be run in order for them to succeed? In this example, Validate() would have to be run before IsValid() or else IsValid() would always return false.

There are exceptions, but in general you should avoid building this kind of API. This is called a "temporal dependency," which is just a fancy term for "one thing is required to happen before the other." The problem with temporal dependencies is that they are seldom self-describing. In other words, the API does not communicate by itself how it is intended to be used. API's that rely on temporal dependencies are often harder to intuitively understand and use than similar code.

In your example, I'd consider refactoring the API with the following design goals:

1. Get rid of IsValid altogther.
2. Validate return a list of validation errors.
3. Redefine "success" as having Validate() return an empty list devoid of errors.

If you simply must have IsValid, redefine it to check #3. If you want to cache the results so that you don't have to recalculate IsValid constantly, consider implementing INotifyPropertyChanged in and invalidating the cached result on PropertyChanged.

Answer 3

Having an interface as return type is good, because your method will be compatible with more types of messages.

Answer 4

Returning interfaces is good in general because it hides the implementation. (Of course, you should not hide more than necessary...)

In the case of your example though, I'm a bit worried that you want to unit-test the actual return type of your function, while the point of using interfaces is precisely to hide it. Looks like your actual code will rely on the actual return type, and that smells bad.

I'd better add an abstract method IsValid() to IMessage, override it in Success and Error, and call validateReturnType.IsValid() directly. Sounds more OO to me. The very goal of OO and virtual methods is to avoid type-based case switching like what you do in your IsValid().