Skip to content
This repository has been archived by the owner on Dec 12, 2020. It is now read-only.

- avoid comparing to null with == #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

srkimir
Copy link

@srkimir srkimir commented May 14, 2016

When new breaker is created via new new CircuitBreaker({...}), self._forced is undefined (not null). Now when _executeCommand gets called there is:

if (self._forced == null) {
  self._updateState();
}

Which will be interpreted as if (undefined == null), which will be true, and library is on safe side. Now imagine that at sometime you will run eslint against this code (http://eslint.org/docs/rules/no-eq-null) and replace above check with if (self._forced === null) (which is anyway, safer than using just ==), whole library would be unusable, right? cause (undefined === null) will lead to false. Problem can easily be solved with if(!self._forced) as proposed in PR, cause it will return true if self._forced is undefined or null.

Other than that there is also proposal to use === instead of == in whole lib, and also using string constants such as OPEN, HALF_OPEN and CLOSED instead of 0, 1, 2, it improves readability, and if you decide to merge PR it is also must have, otherwise when self._forced is equal to 0, than
if (!self._forced) would be interpreted as true because of if(!0)

Tests results with proposed changes:

Running "jshint:all" (jshint) task
>> 2 files lint free.

Running "jasmine:all" (jasmine) task
Testing jasmine specs via phantom
..........................
26 specs in 0.011s.
>> 0 failures

Done, without errors.

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

Successfully merging this pull request may close these issues.

1 participant