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

Provide Validation API that does not use exceptions #32

Closed
jjathman opened this Issue Oct 13, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@jjathman
Contributor

jjathman commented Oct 13, 2014

I believe it would make for more usable API if the FhirValidator class would return some sort of ValidationResult object rather than returning void but throwing exceptions if there is a validation issue. Handling exceptions for something as generally expected as validation errors seems to create less code which is more difficult to follow. There is some good explanation here about why this is an anti-pattern.

If this was an intentional design decision then we will use the existing API. If you would like to see an alternative API I would be more than happy to create a pull request for adding a new API for validation errors.

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Oct 14, 2014

Hi Joe,

To be totally honest, the only reason it was designed like that is because the HAPI v2 API's validation library does it that way. I think you're right, there are lots of valid reasons not to repeat that mistake. If you wanted to create a pull request which replaces the exceptions with some sort of ValidationResult object I think it would make a lot of sense to merge.

@jjathman

This comment has been minimized.

Contributor

jjathman commented Oct 14, 2014

That sounds good. Would you like me to keep the existing API and deprecate it or just modify it in place since we are pre-1.0?

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Oct 14, 2014

I suppose keep and deprecate would be ideal. I've been trying to avoid too
many breaking changes without at least one release to adjust... :)

sent from my phone.
On Oct 14, 2014 5:37 PM, "Joe Athman" notifications@github.com wrote:

That sounds good. Would you like me to keep the existing API and deprecate
it or just modify it in place since we are pre-1.0?


Reply to this email directly or view it on GitHub
#32 (comment).

@jjathman

This comment has been minimized.

Contributor

jjathman commented Oct 15, 2014

That's a good strategy. I'll put together a pull request tomorrow. Should be easy to change. Just a small change needed I think. Thanks.

jamesagnew added a commit that referenced this issue Oct 17, 2014

Merge pull request #34 from jjathman/remove-validation-exception
Fixes #32 - removes new validation API that doesn't use exceptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment