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

feat: validate custom srcsets #61

Merged
merged 3 commits into from
Jun 5, 2020
Merged

feat: validate custom srcsets #61

merged 3 commits into from
Jun 5, 2020

Conversation

ericdeansanchez
Copy link
Contributor

This PR implements basic validation for start, stop, tol, and
widths.

When invalid input is encountered an exception will be thrown.

Specifically, a InvalidArgumentException is thrown detailing the error
state that has occurred.

We debated whether or not to throw exceptions in a few cases;
ultimately we have decided that error states should be clearly
defined and handled (both for us internally and for users).

What this means is that, in a few cases, our library will throw
which results in callers being immediately alerted of the error
state that caused the exception to be thrown (so that errors are
not propagated silently throughout application code).

This is an added layer of safety to help ensure code that depends
on this library remains correct (w.r.t. it's usage of this library).

In short, the validation we are doing here says that callers:

  • cannot define negative image widths
  • cannot define invalid width ranges, e.g. start > stop
  • cannot define invalid width tolerance values, e.g. 0.00001

We consider these constraints to be reasonable and empowering
rather than limiting. That is, this kind of validation helps guard
against typos, e.g.

Copy link
Contributor

@sherwinski sherwinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@ericdeansanchez ericdeansanchez merged commit 6f7b6af into master Jun 5, 2020
@ericdeansanchez ericdeansanchez deleted the validation branch June 5, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants