Closed JN-Jones closed 9 years ago
Any comment?
Yep, looks right to me :) Any reason you removed the handler functions though?
Note: The functions in the exception handler are the same as in the parent handler so there wasn't any need for them.
The parent class has exactly the same checks. Should we need them we can easily readd them later.
Ah, must have missed that. Otherwise, this looks perfectly fine to me. I won't merge it, so that you can do any other instances in the same PR.
I think I've found all exceptions. As soon as this get's merged I'm going to change the packages too.
@euantorano @wpillar Can we merge this?
Sure, only comment is that the only thing that's different between all of the new Exception classes is protected $message = 'errors.topic_not_found';
the message variable.
Could abstract the constructor to make it easier to create new Exceptions of this nature, but this shouldn't block a merge. Just a thought.
The main Problem is that the constructors are all different. Exception
and RuntimeException
have the same, NotFoundHttpException
has another, the other Http Exceptions have also different etc. We could add AbstractTranslatable*Exception
s but we would end with multiple ones for different exceptions. Also this way we can easily add custom things like I did with the captcha Exception.
Cool :)
63
Before I start adding exceptions everywhere I would like to know whether this is what you want or whether you want something else @euantorano @wpillar
Note: The functions in the exception handler are the same as in the parent handler so there wasn't any need for them.