Skip to content

Library is using Asserts without throwing a message #21

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

Closed
jposada202020 opened this issue Apr 26, 2021 · 7 comments
Closed

Library is using Asserts without throwing a message #21

jposada202020 opened this issue Apr 26, 2021 · 7 comments

Comments

@jposada202020
Copy link
Contributor

jposada202020 commented Apr 26, 2021

Library is using assert statements. It is preferable to add a customary message to inform the problem
outdated: .instead use Exceptions.

@dastels
Copy link
Collaborator

dastels commented Apr 26, 2021

I disagree. Asserts are good for catching cases that can not sanely occur: programmer error, while exceptions are best suited to things a that go wrong unexpectedly, but for which there is often a way to deal with it: e.g. a communication problem. I.e. Exceptions can be handled, but asserts have no recourse but to cause execution to stop/fail.

In this case the assert enforces the contract of the method... gyro_range HAS to be one of those values. If it's something else, the programmer messed up.

@jposada202020
Copy link
Contributor Author

Thanks, valid points. not arguing with that. Taking into account, that I have no problem closing this issue, what would be more helpful to the end user?
maybe and assertion with a message could work?

@dastels
Copy link
Collaborator

dastels commented Apr 26, 2021

It's really a matter of accepted/encouraged style. But that's what asserts are for. Some people use them, some don't. Using an exception and not handling it does the same job, but obscures the intent.

A message is always a good idea.

@jposada202020
Copy link
Contributor Author

Right Thanks.. 👍🏼

@jposada202020 jposada202020 changed the title Library is using Asserts Library is using Asserts without throwing a message Apr 26, 2021
@tannewt
Copy link
Member

tannewt commented Apr 26, 2021

How does one add a message to an assert in case it happens? If it's not expected to happen then I'd rather remove them. (Remember that asserts use memory.) If they may happen, then an exception with a message would make them more actionable.

@jposada202020
Copy link
Contributor Author

Will do Thanks

@jposada202020
Copy link
Contributor Author

Fix by #23

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

3 participants