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

Using assert to indicate error #30

Closed
NVolcz opened this Issue Aug 25, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@NVolcz
Contributor

NVolcz commented Aug 25, 2016

When parsing a broken XML document the following assertion failure is generated. Since assertion failure is non-recoverable an exception seems like better for the default.
´´´
core.exception.AssertError@../../../../.dub/packages/std-experimental-xml-master/std-experimental-xml/source/std/experimental/xml/cursor.d(500): Assertion failure
´´´

@lodo1995

This comment has been minimized.

Show comment
Hide comment
@lodo1995

lodo1995 Aug 25, 2016

Owner

The error handler defaults to assert(0), but is configurable. You can find an example in the documentation (look at the third example).

Just use myParser.cursor(myErrorHandler) to have a cursor that does whatever you want in case of error.

Owner

lodo1995 commented Aug 25, 2016

The error handler defaults to assert(0), but is configurable. You can find an example in the documentation (look at the third example).

Just use myParser.cursor(myErrorHandler) to have a cursor that does whatever you want in case of error.

@lodo1995 lodo1995 closed this Aug 27, 2016

@NVolcz

This comment has been minimized.

Show comment
Hide comment
@NVolcz

NVolcz Aug 27, 2016

Contributor

I meant that asserts are not supposed to signal errors on user input

On Sat, Aug 27, 2016, 17:08 Lodovico Giaretta notifications@github.com
wrote:

Closed #30 #30.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#30 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABlyaqaGrHEfgdWqxnKT4cwODIyn6x9Lks5qkFLlgaJpZM4JtUlJ
.

Contributor

NVolcz commented Aug 27, 2016

I meant that asserts are not supposed to signal errors on user input

On Sat, Aug 27, 2016, 17:08 Lodovico Giaretta notifications@github.com
wrote:

Closed #30 #30.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#30 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABlyaqaGrHEfgdWqxnKT4cwODIyn6x9Lks5qkFLlgaJpZM4JtUlJ
.

@lodo1995 lodo1995 reopened this Aug 27, 2016

@lodo1995

This comment has been minimized.

Show comment
Hide comment
@lodo1995

lodo1995 Aug 27, 2016

Owner

Sorry. I may have misunderstood your point. The reason I decided to use asserts as the default callback is that they indicate code that should never be reached. This is the case, as if you didn't put an handler it means you are expecting your input to always be correct (it may not come from the user, but from an application which you trust).
If you know your input cannot be trusted, and want your application to be fault-tolerant, you should definitely put your own handler there.
But this is just my point of view, I may be wrong on what's better as a default. So I'll probably bring this to the attention of the reviewers, so I can have their feedback on this.
Thank you very much for giving your opinion.

Owner

lodo1995 commented Aug 27, 2016

Sorry. I may have misunderstood your point. The reason I decided to use asserts as the default callback is that they indicate code that should never be reached. This is the case, as if you didn't put an handler it means you are expecting your input to always be correct (it may not come from the user, but from an application which you trust).
If you know your input cannot be trusted, and want your application to be fault-tolerant, you should definitely put your own handler there.
But this is just my point of view, I may be wrong on what's better as a default. So I'll probably bring this to the attention of the reviewers, so I can have their feedback on this.
Thank you very much for giving your opinion.

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