Exceptions throwing for invalid argument inside a method

Question

I'm learning about Exception handling. I already know how to use them, however I don't know when to use them, since few tutorials tell you anything insightful about this. My code:

// 0-index part of the url
public function part($Part)
  {
  if (!is_numeric($Part))
    throw new Exception('The argument for $Url->part() should be numeric');

  $Part = (int) $Part;

  if ($Part < 0)
    throw new Exception('The argument for $Url->part() should be positive');

  if ($Part > count($this->parts))
    return false;

  return $this->parts[$Part];
  }

I feel that my code has too many exceptions. It's part of a code for retrieving the current url and some parts of them. e.g., /this/is/a/test/ would be saved in $this->parts as array('this', 'is', 'a', 'test').

Am I using too many exceptions in that method, hindering the readability? Should I use only one exception if any of the problems arise, making it slightly more difficult to debug but easier to read the source code?

This is the more generic exception named in the question:

// 0-index part of the url
public function part($Part)
  {
  if (!is_numeric($Part) || intval($Part) < 0 || intval($Part) > count($this->parts))
    throw new Exception('The argument for $Url->part() is not correct');

  return $this->parts[(int) $Part];
  }

Answer 1

Actually, your question is very close to opinion-based questions (thus it may be too hard to answer properly, without non-constructive discussion). However, there are some things that are useful to remember.

First, raising just exception may be improved - because there are standard exceptions in PHP. Throwing them will defenitely improve readability. For example, your code accept some argument that supposed to be numeric. If it isn't - then it's invalid argument - therefore, corresponding exception may be triggered. Next, your parts is an array - and you want to check if passed offset exists. If it isn't, then it's out-of-bounds exception. So your code may look like:

public function getPart($part)
{
   if(!is_numeric($part))
   {
      throw new InvalidArgumentException('Invalid part number passed');
   }
   if(!array_key_exists($part, $this->parts))
   {
      throw new OutOfBoundsException('Offset '.$part.' not found in parts');
   }
   return $this->parts[$part];
}

-this seems to be reasonable for numeric offsets. However, if your structure contains more complex data, then you may want to raise logic exception (to point that there was error in logical structure of passed arguments or current structure).

In common case - it all depends of situation and logic. There's no silver bullet - it's about design and all solutions are relative and bound to certain situation.

Again, this is very opinion-based (for example, I started from re-naming your method to getPart() - because for me method is an entity that should be named as action name, not just thing name). And, more - try to avoid mixing different type when returning something (like false in your sample). It causes unreliable behavior. Better to throw exception, but keep your function/method return type same.

Answer 2

@Mark, it isn't user input what I want to validate, it's the programmer's input.

Yes, that's overkill. If you want to sanity-check programmer input, user assert.

There is no reasonable way to handle an exception generated by mistyped code, or insane arguments, or incorrect use of a library, which is what you seem to be guarding against. There is no value to using exceptions as the mechanism to guard against such errors.

You raise an exception when something unexpected happens, and you can't deal with it gracefully. Exceptions pass control back up the stack, ideally to a level where that type of error can be handled, without the program failing. There is really no way at runtime to handle the kind of error you're guarding against. If somebody gives your library inputs that are so wrong that your library can only abort, the best thing we can do at that point is give the user as much debugging information as possible and exit; assert is purpose-built for this.

Note that I think even assert is usually over-kill. You're using a duck-typed language; who cares about the type of your arguments? So long as they respond to all the methods you're going to be invoking on them, your code should be happy to accept any kind of object. The only case I would consider using assert on my arguments is when I'm writing library code and I know I want to produce friendly error messages than PHP's.