-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to a custom exception instead of InvalidArgumentException #202
Comments
There are other issues/requests that are breaking BC #197 So I think it would be reasonable to create a milestone to collect all in one place. |
This library could provide a custom exception without it being a BC break. The custom exception would just have to extend |
@BackEndTea it's already possible to do that, but it's not ideal. The point is, there is hardly any reason for the library to use InvalidArgumentException. |
There is: it's an assertion library meant to add guards for things the PHP type system is lacking. This is not a validation library, because as soon as you want that you start to dwell on:
None of this is trivial and would make this library a whole new beast. I understand it may be frustrating to see redundancy in some assertions, but the goal and the way it works is very different. |
/cc @andrew-demb @Ocramius as I think this concerns #222 too |
Worth mentioning: I'm saying this as my & webmozart opinion. We however left the repository in the hands of @BackEndTea so obviously if Gert wishes to change the direction of the library to take this path then so be it |
No need for a BC break - can extend Can be done in a minor release. |
@Ocramius in this case, the logic of your application would need to be able to distinguish between InvalidArgumentException and its descendent used by the Assert library. Hardly the best of two worlds. A proper solution is to have a custom exception that based on the context of an assertion, could be translated to InvalidArgumentException if needed, but in the majority of cases could be left as is. A process of asserting is not always about checking some arguments that may end up being invalid. Asserting -> AssertException. |
The BC break ia still there though. Additional inheritance level may not be the best scenario, but it is better than breaking compatibility. |
This library should have its own AssertException to avoid a need to:
I realize this a BC break, but this issue is too serious to keep ignoring. At least a few tickets related to this problem were closed without any resolution and people will continue fighting it again in the future.
The text was updated successfully, but these errors were encountered: