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

Replace asserts with exceptions #53

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Replace asserts with exceptions #53

wants to merge 2 commits into from

Conversation

Rupan
Copy link
Contributor

@Rupan Rupan commented Jun 3, 2019

WIP/RFC: this pull request is not in a mergeable state! I want feedback from the library maintainers regarding whether it would be considered for merge if/when it is completed before investing additional time.

In its current state, the EBML library can operate in two modes:

  1. Built in debug mode, the library will cause application crashes when an assertion is hit
  2. Built in release mode, critical errors will be silently ignored, possibly leading to strange application crashes

I propose that the assertions in the library be changed to a custom exception class which can provide additional context regarding what expectations were not met. This will work for both modes (1) and (2) above and allow application maintainers to better control how errors are handled.

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

Successfully merging this pull request may close these issues.

2 participants