feat: add isValidNumber, isNotValidNumber #247

Undistraction · 2017-12-20T19:48:34Z

adds isValidNumber which tests whether a value is a number that is not either NaN, Infinity or -Infinity.
adds isNotValidNumber which is the complement of isValidNumber.
adds isInvalidNumber which is aliased to isNotValidNumber.

Please note the following:

Typescript

I haven't added the three new functions to index.d.ts as I have no familiarity with TypeScript and I would be doing so blindly.

Docs

I have set the @since versions at X.X.X as I don't know what version you will include this feat in.

Tests

npm test

I have one failing test which was already failing when I pulled the branch and is unrelated to my changes. If you want me to dig deeper please let me know and I'll see if I can work out why it's failing on my machine:

isPromise when value is an instance of native Promise should return true:

      AssertionError: 'false' === 'true'
      + expected - actual

      -false
      +true

npm test:ramda

Initally failed because of unmet peer dependency of ajv. I upgraded ajv to @5 and all pass other than the test outlined above.

Ref char0n#235

char0n · 2017-12-20T20:09:10Z

src/index.js

@@ -53,6 +53,9 @@ export { default as isInteger } from './isInteger';
 export { default as isNotInteger } from './isNotInteger';
 export { default as isFloat } from './isFloat';
 export { default as isNotFloat } from './isNotFloat';
+export { default as isValidNumber } from './isValidNumber';
+export { default as isNotValidNumber } from './isNotValidNumber';
+export { default as isInvalidNumber } from './isNotValidNumber'; // alias of isNotValidNumber


IMHO this is somehow misleading naming for isInvalidNumber. The name indicates that the value checked by the predicate will be number, but not a valid one e.g. NaN, Infinity.

I would imagine isInvalidNumber to have the following signature

const isInvalidNumber = both(isNumber, isNotValidNumber);

OK. Agreed so:

Do we need an alias for isNotValidNumber? I think probably not.

Do we want an isInvalidNumber function that only returns true for NaN, Infinity and -Infinity? I'd say it's probably pretty esoteric.

char0n · 2017-12-20T20:09:43Z

src/isNotValidNumber.js

+import { complement } from 'ramda';
+import isValidNumber from './isValidNumber';
+
+/* eslint-disable max-len */


Drop this. Your lines length should not go over 100 chars

char0n · 2017-12-20T20:09:52Z

src/isNotValidNumber.js

+ * RA.isNotValidNumber(1); //=> false
+ * RA.isNotValidNumber(''); //=> true
+ */
+/* eslint-enable max-len */


Drop this. Your lines length should not go over 100 chars

Not sure what you mean. What should I drop?

/* eslint-enable max-len */

Ah. Sorry. Thought you'd wrapped all your jsdoc comment blocks in the enable/disable comments, but was just the one I cribbed from.

char0n · 2017-12-20T20:11:04Z

src/isNotValidNumber.js

+ *
+ * @func isNotValidNumber
+ * @memberOf RA
+ * @since {@link https://char0n.github.io/ramda-adjunct/X.X.X|vX.X.X}


/2.2.0/v2.2.0

char0n · 2017-12-20T20:11:49Z

src/isNotValidNumber.js

@@ -0,0 +1,24 @@
+import { complement } from 'ramda';


divide absolute and relative imports by one empty line pls

char0n · 2017-12-20T20:36:56Z

src/isValidNumber.js

+ *
+ * @func isValidNumber
+ * @memberOf RA
+ * @since {@link https://char0n.github.io/ramda-adjunct/X.X.X|vX.X.X}


char0n · 2017-12-20T20:37:24Z

src/isValidNumber.js

+ * @memberOf RA
+ * @since {@link https://char0n.github.io/ramda-adjunct/X.X.X|vX.X.X}
+ * @category Type
+ * @sig * -> Boolean


Add

@see {@link RA.isNotValidNumber|isNotValidNumber}

char0n · 2017-12-20T20:37:39Z

test/isNotValidNumber.js

+import eq from './shared/eq';
+import args from './shared/arguments';
+import Symbol from './shared/Symbol';
+


insert empty line

char0n · 2017-12-20T20:38:08Z

test/isNotValidNumber.js

+  });
+});
+
+describe('isInvalidNumber', function() {


Please drop this one. We'll not deal with isInvalidNumber in this PR.

char0n · 2017-12-20T20:38:21Z

test/isValidNumber.js

+import eq from './shared/eq';
+import args from './shared/arguments';
+import Symbol from './shared/Symbol';
+


insert another empty line

char0n · 2017-12-20T20:41:29Z

Regarding the isPromise not passing. I'am not able to reproduce. That is really really strange because these tests are running inside node.

char0n · 2017-12-20T20:43:41Z

Typescript signatures will looks like this:

/**
* ... description
*/
isValidNumber(val: any): boolean;

/**
* ... description
*/
isNotValidNumber(val: any): boolean;

char0n · 2017-12-20T20:49:31Z

src/isValidNumber.js

+ * @return {Boolean}
+ * @example
+ *
+ * RA.isValidNumber(1); //=> true


I'd add examples also for NaN, Infinity and -Infinity

char0n · 2017-12-20T20:49:43Z

src/isValidNumber.js

+ * RA.isValidNumber(1); //=> true
+ * RA.isValidNumber(''); //=> false
+ */
+/* eslint-enable max-len */


Drop this. Your lines length should not go over 100 chars

Ref char0n#247

Undistraction · 2017-12-22T17:19:47Z

All changes made and I have removed alias isInvalidNumber.

char0n

LGTM. Good job and thank you for the contribution!

Undistraction added 3 commits December 20, 2017 16:26

feat: add isValidNumber, isNotValidNumber

c6703ef

Ref char0n#235

feat: add isInvalidNumber as alias for isNotValidNumber

056dedd

docs: remove Number namespace from infinity vars

70dd9de

Undistraction changed the title ~~Feat/is valid number~~ Feat/isValidNumber Dec 20, 2017

Undistraction changed the title ~~Feat/isValidNumber~~ add: isValidNumber, isNotValidNumber, isInvalidNumber Dec 20, 2017

Undistraction changed the title ~~add: isValidNumber, isNotValidNumber, isInvalidNumber~~ feat: add isValidNumber, isNotValidNumber, isInvalidNumber Dec 20, 2017

char0n requested changes Dec 20, 2017

View reviewed changes

char0n reviewed Dec 20, 2017

View reviewed changes

char0n added the feature label Dec 20, 2017

char0n added this to the v2.2.0 milestone Dec 20, 2017

Undistraction changed the title ~~feat: add isValidNumber, isNotValidNumber, isInvalidNumber~~ feat: add isValidNumber, isNotValidNumber Dec 22, 2017

refactor(isValidNumber): Requested changes for PR

d4b873a

Ref char0n#247

fix(IsNotValidNumber): Add newline at end of file

a4f7524

char0n approved these changes Dec 22, 2017

View reviewed changes

char0n merged commit d94c1b5 into char0n:master Dec 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add isValidNumber, isNotValidNumber #247

feat: add isValidNumber, isNotValidNumber #247

Undistraction commented Dec 20, 2017 •

edited

Loading

char0n Dec 20, 2017

Undistraction Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

Undistraction Dec 20, 2017

char0n Dec 20, 2017

Undistraction Dec 20, 2017 •

edited

Loading

char0n Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

char0n commented Dec 20, 2017

char0n commented Dec 20, 2017

char0n Dec 20, 2017

char0n Dec 20, 2017

Undistraction commented Dec 22, 2017 •

edited

Loading

char0n left a comment

feat: add isValidNumber, isNotValidNumber #247

feat: add isValidNumber, isNotValidNumber #247

Conversation

Undistraction commented Dec 20, 2017 • edited Loading

Typescript

Docs

Tests

npm test

npm test:ramda

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Undistraction Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

char0n commented Dec 20, 2017

char0n commented Dec 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Undistraction commented Dec 22, 2017 • edited Loading

char0n left a comment

Choose a reason for hiding this comment

Undistraction commented Dec 20, 2017 •

edited

Loading

Undistraction Dec 20, 2017 •

edited

Loading

Undistraction commented Dec 22, 2017 •

edited

Loading