Skip to content
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

Consider more specific exceptions for validation methods #76

Closed
ndrwrbgs opened this issue Dec 2, 2017 · 6 comments
Closed

Consider more specific exceptions for validation methods #76

ndrwrbgs opened this issue Dec 2, 2017 · 6 comments

Comments

@ndrwrbgs
Copy link
Contributor

ndrwrbgs commented Dec 2, 2017

E.g. ArgumentNotLessThanException with fields that expose the actual value and the validated against value.

@ndrwrbgs
Copy link
Contributor Author

ndrwrbgs commented Dec 2, 2017

v8 Breaking change

@danielwertheim danielwertheim added this to the v8 milestone Jan 19, 2018
@danielwertheim danielwertheim removed this from the v8 milestone Feb 13, 2018
@danielwertheim
Copy link
Owner

Not really sure I see the value in adding custom exception types. What will you do with the extra information lying in dedicated properties?

@ndrwrbgs
Copy link
Contributor Author

ndrwrbgs commented Feb 14, 2018

The benefit would be in providing more information upstream about the failure - most of the top nuget packages provide custom exception types since c# unlike java does not provide a super rich set of exceptions.

A couple uses I can see:

  • customization of exception messages if they need to wrap the one thrown from here
  • ease of structured logging, no string parsing to get the bounds needed
  • in cases where the value is T (like Contains), a ToString() might not represent the value sufficiently (they’ll get “expected element List’1<System__Canon>” kind of messages that they aren’t able to rectify).

All in all these aren’t :vital: and are rather edge cases but if we are throwing anyway I’d rather take an “enterprise” style approach and support as many use cases as possible (where supporting those cases does not add substantial work at least)

@danielwertheim
Copy link
Owner

v8.0.0 is going out nowish, but this isn't really breaking as I see it as it would still be able to extend the current exceptions being thrown in the specific cases. If so, then it can be added whenever.

@ndrwrbgs
Copy link
Contributor Author

ndrwrbgs commented Feb 14, 2018 via email

@ndrwrbgs
Copy link
Contributor Author

It looks like you were okay with the proposal above. I will consider this item unblocked (this is my non-committal way of saying I'll work on it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants