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

InvalidOperatorType Exception: improve documentation, duplicate code and unit test #62

Merged
merged 1 commit into from
May 28, 2018

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Mar 26, 2018

The $validOperators were defined in two places, making the chance of these two arrays getting out of sync large and lowering maintainability.

I've now:

  • Removed the $validOperators property in the Whip_InvalidOperatorType class in favour of adding a new parameter to the constructor.
  • The one call to the Exception will now pass the operators against which it validates.
  • The message text of the Exception is now unit tested.

…and unit test

The `$validOperators` were defined in two places, making the chance of these two arrays getting out of sync large and lowering maintainability.

I've now:
* Removed the `$validOperators` property in the `Whip_InvalidOperatorType` class in favour of adding a new parameter to the constructor.
* The one call to the Exception will now pass the operators against which it validates.
* The message text of the Exception is now unit tested.
@jcomack jcomack self-assigned this May 28, 2018
Copy link
Contributor

@jcomack jcomack left a comment

Choose a reason for hiding this comment

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

CR done 👍

@jcomack
Copy link
Contributor

jcomack commented May 28, 2018

Acceptance done 👍

@jcomack jcomack merged commit e06c8cb into master May 28, 2018
@jcomack jcomack added this to the Next release milestone May 28, 2018
@jcomack jcomack deleted the JRF/duplicate-code-maintainability branch May 28, 2018 08:34
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