Design Decision: Communicating from a Method

Question

This is a simple design decision that seems to have significant passions on each side. I am struggling to really understand which design has the least negative consequences.

I have a method to add a banana:

public Banana AddBanana(string name) 
{
    // add the banana
    var _Banana = new Banana { Name = name };
    this.Bananas.Add(_Banana);
    return _Banana;
}

But in some cases I cannot add a banana, something like this:

public Banana AddBanana(string name) 
{
    // test the request
    if (this.Bananas.Count > 5)
        return null;
    if (this.Bananas.Where(x => x.Name == name).Any())
        return null;
    // add the banana
    var _Banana = new Banana { Name = name };
    this.Bananas.Add(_Banana);
    return _Banana;
}

Now I want to communicate back to the caller WHY they can't.

WHICH way is the better approach?

Approach 1: communicate with an exception

public Banana AddBanana(string name) 
{
    // test the request
    if (this.Bananas.Count > 5)
        throw new Exception("Already 5 Bananas");
    if (this.Bananas.Where(x => x.Name == name).Any())
        throw new Exception("Banana Already in List");
    // add the banana
    var _Banana = new Banana { Name = name };
    this.Bananas.Add(_Banana);
    return _Banana;
}

Approach 2: communicate with a test

public Class CanAddBananaResult 
{
    public bool Allowed { get; set; }
    public string Message { get; set; }
}

public CanAddBananaResult CanAddBanana(string name) 
{
    // test the request
    if (this.Bananas.Count > 5)
        return new CanAddBananaResult { 
            Allowed = false, 
            Message = "Already 5 Bananas" 
        };
    if (this.Bananas.Where(x => x.Name == name).Any())
        return new CanAddBananaResult { 
            Allowed = false, 
            Message = "Banana Already in List" 
        };
    return new CanAddBananaResult { Allowed = true };
}

public Banana AddBanana(string name) 
{
    // test the request
    if (!CanAddBanana(name).Allowed)
        throw new Exception("Cannot Add Banana");
    // add the banana
    var _Banana = new Banana { Name = name };
    this.Bananas.Add(_Banana);
    return _Banana;
}

In Approach 1, the consumer knows the problem based on the exception.Message.

In Approach 2, the consumer can prevent the exception rather than catch it.

Which approach is better overall?

I read this: Design classes so that an exception is never thrown in normal use. For example, a FileStream class exposes another way of determining whether the end of the file has been reached. This avoids the exception that is thrown if you read past the end of the file. http://msdn.microsoft.com/en-us/library/seyhszts(v=vs.71).aspx

But the "exceptional" approach seems to be less code. Does that mean simpler/better?

Answer 1

The second approach seems unnecessarily complex, so I tend to prefer the first one.

To give the caller the opportunity to perform the checks himself, I'd add the following members:

public bool IsFull
{
    get { return this.Bananas.Count > 5; }
}

public bool ContainsBanana(string name)
{
    return this.Bananas.Any(b => b.Name == name);
}

I'd probably also return the existing banana if the name already exists:

public Banana GetOrAddBanana(string name) 
{
    var banana = this.Bananas.FirstOrDefault(b => b.Name == name);
    if (banana == null)
    {
        if (this.IsFull) throw new Exception("Collection is full");
        banana = new Banana { Name = name };
        this.Bananas.Add(banana);
    }
    return banana;
}

If not being able to add a banana is not an exceptional case, but can occur during normal operation, I'd use the Try... pattern:

public bool TryGetOrAddBanana(string name, out Banana banana) 
{
    banana = this.Bananas.FirstOrDefault(b => b.Name == name);
    if (banana == null)
    {
        if (this.IsFull) return false;
        banana = new Banana { Name = name };
        this.Bananas.Add(banana);
    }
    return true;
}