Trying to add a duplicate to a set - what kind of exception to throw?

Question

I'm creating a method on a Doctrine model to add related objects to a collection, but I want to throw an exception when a duplicate object is added to that collection.

Here's the test(s):

public function testFluentInterface(  )
{
  $sport = new Sport();
  $this->assertSame($sport, $sport->addCode('ANY'),
    'Expected Sport to implement fluent interface.'
  );
}

public function testCannotAddSameCodeMoreThanOnce(  )
{
  $code = 'BAZ';

  $sport = new Sport();
  $sport->addCode($code);

  try
  {
    $sport->addCode($code);
    $this->fail(
      'Expected error when trying to add the same code to a sport more than once.'
    );
  }
  catch( /*SomeKindOf*/Exception $e )
  {
  }
}

At first, I thought it might be appropriate for an OverflowException to be thrown in this case, but I'm not certain whether "this value already exists" is the same as "this container is full":

Exception thrown when you add an element into a full container.

There's UnexpectedValueException, but that seems to be more applicable to variables with incorrect types:

Exception thrown if a value does not match with a set of values. Typically this happens when a function calls another function and expects the return value to be of a certain type or value not including arithmetic or buffer related errors.

I could always use LogicException, but that seems a little generic for this use case:

Exception that represents error in the program logic. This kind of exceptions should directly lead to a fix in your code.

Is there a better choice here? Are my observations correct? What would be the most appropriate SPL exception to throw when trying to add a duplicate to a collection that must contain unique values?

Answer 1

The more I think about it – especially in light of Toni's and Francis' respective answers – the more I wonder if throwing an exception is useful in this scenario.

Per Toni's point, many classes already handle this situation differently. Although returning a "failure" value is not an option in this case (the Sport class implements a fluent interface), attempting to add an object to a collection twice is not necessarily an "exceptional" case.

Francis also made a good point that other constructs in PHP – such as arrays - do not care about this sort of thing, either. Throwing an exception when a duplicate item is added to a collection would be unprecedented behavior that could lead to maintainability problems down the road.

Also, throwing an exception could be considered to be at odds with the concept of fluent interface. What would be the point of allowing the developer to chain methods on an instance if he has to break up the chain with conditional checks anyway?

In other words, if the goal is this:

$sport
  ->addCode($codeA)
  ->addCode($codeB)
  ->setSomeOtherProperties(...)
  ->save();

Then it makes no sense to force the developer to have to do this:

if( ! $sport->hasCode($codeA) )
{
  $sport->addCode($codeA);
}
// etc. - Why bother having a fluent interface if it's unsafe to use it?

The above code is particularly problematic because addCode() is going to call hasCode() anyway, which in effect forces the developer to invoke duplicate computation.

And using a try/catch isn't much better:

try
{
  $sport
    ->addCode($codeA)
    ->addCode($codeB)
    ->setSomeOtherProperties(...)
    ->save();
}
catch( LogicException $e )
{
  // $sport is now in an unknown state.
}

Trying to recover from a LogicException in the above block is impractical, and it shouldn't even be necessary here; trying to add a code twice shouldn't be enough to stop the Sport object from being persisted.

Answer 2

There's no SPL exception that really fits. You might be better off creating your own Exception:

/**
 * Raised when item is added to a collection multiple times. 
 */
class DuplicateException extends RuntimeException {}; 

Throwing an exception in this case seems a bit drastic, somewhat equivalent to throwing an exception whenever you assign the same value to the same key in an array. Have you considered using a return value instead of an exception to detect this case?

Answer 3

As an example many classes in the Java Collection Framework (i.e. Set) do not throw an exception in such cases, they return false.

Personally I would go that way.