-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add isNotNil #2818
Add isNotNil #2818
Conversation
This sounds like a good idea. We probably should have done this long ago. I'm not certain of the name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't commit ramda.js
or ramda.min.js
.
Also, most importantly, this does not include a test file. Please look at the test folder for examples.
source/isDefined.js
Outdated
* R.isDefined(0); //=> true | ||
* R.isDefined([]); //=> true | ||
*/ | ||
var isDefined = complement(isNil); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no reason to depend on complement
here. Just do it manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in 0789ed3
if notNil(person) { /* ... */ };
if isNotNil(person) { /* ... */ };
if defined(person) { /* ... */ };
if isDefined(person) { /* ... */ };
if exists(person) { /* ... */ }; I don't mind being a bit more expressive instead of using |
In function names, I like to prevent "not", because in my opinion it is an unnecessary mental overhead and in the past often created unnecessary confusions for me. Also I think isNotNil exposes an implementation detail. It could also be specified e.g. that "" is "!isDefined" as well (which then probably would be "isTurthy" or smth). I'd really like to choose a "postive" name instead of doing double negation using words. (not isNot is yes) Greetings |
|
I don't share your objections to using 'not' in the name; So does anyone have a another candidate for a name for this function? @ramda/core? |
I changed the function name to isNotNil. |
38065ee
to
41cea32
Compare
Why not use ramda adjunct? I know that it adds a lot o functions.. but it has most of the functions that i find myself repeating across projects using ramda Btw: I thought that ramda was meant to have only "base" functional tools, that's why i think that we still don't have the "isNot*" functions. If I'm wrong, would be nice to have |
And also there is a https://ramda-extension.firebaseapp.com/docs/#isNotNil |
source/index.js
Outdated
@@ -98,6 +98,7 @@ export { default as invert } from './invert'; | |||
export { default as invertObj } from './invertObj'; | |||
export { default as invoker } from './invoker'; | |||
export { default as is } from './is'; | |||
export { default as isDefined } from './isNotNil'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed in renaming.
source/isNotNil.js
Outdated
* R.isDefined(null); //=> false | ||
* R.isDefined(undefined); //=> false | ||
* R.isDefined(0); //=> true | ||
* R.isDefined([]); //=> true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed in renaming.
test/isNotNil.js
Outdated
var R = require('../source'); | ||
var eq = require('./shared/eq'); | ||
|
||
describe('isDefined', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed in renaming.
42a53b2
to
a216b5c
Compare
I'm not sure about this one, I did use to use something similar when first starting out with Ramda but then found it to be unnecessary: const guardFn = when(isDefined);
// same as:
const guardFn = unless(isNil); And: const byDefined = filter(isDefined);
// same as:
const byDefined = reject(isNil); I'd say 99% of the time I would want to do a null check, I'd use one of these two patterns. |
In javascript projects it is often necessary to to someething if something is not nil. This commits adds a function "isNotNil" and its corresponding test which does the null and undefined checking.
a216b5c
to
befbbef
Compare
@LarsKoelpin @CrossEye This pull request could not be merged because of a merge conflict with If we still want this, this should now be ready to go. |
I feel like writing complement(isNil) a lot to do null/undefned checking.
I haven't found an appropriate alternative, so I think it might be a good addition to the library to add
a function named
isDefined: Object -> Bool
which does the null/undefined checking.
What do you think of this addition?