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

Add isNotNil #2818

Merged
merged 2 commits into from Jan 22, 2022
Merged

Add isNotNil #2818

merged 2 commits into from Jan 22, 2022

Conversation

LarsKoelpin
Copy link
Contributor

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?

@CrossEye
Copy link
Member

This sounds like a good idea. We probably should have done this long ago. I'm not certain of the name. isNotNil seems more likely to me.

Copy link
Member

@CrossEye CrossEye left a 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.

* R.isDefined(0); //=> true
* R.isDefined([]); //=> true
*/
var isDefined = complement(isNil);
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 0789ed3

@bas080
Copy link

bas080 commented May 12, 2019

This sounds like a good idea. We probably should have done this long ago. I'm not certain of the name. isNotNil seems more likely to me.

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 not.

@LarsKoelpin
Copy link
Contributor Author

LarsKoelpin commented May 12, 2019

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.
That's why I consider not in names an antipattern and try to name things in a "positive" way.

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)
I will be adding the test and incorporating your changes soon when I'm back home again.

Greetings

@LarsKoelpin
Copy link
Contributor Author

  • Testcase Added: Please review if the specification matches your thoughts
  • Removed dist/ commit from history
  • Removed dependecy on comeplemt

@CrossEye
Copy link
Member

I don't share your objections to using 'not' in the name; nil to me means simply having one of the two nil types in JS, null or undefined. But that was just a quick suggestion; isNotNil is not a name I care about. But isDefined does not resonate for me. I didn't guess what it did from the name. When I saw the title, I thought that maybe you were simply looking for R.has.

So does anyone have a another candidate for a name for this function? @ramda/core?

@LarsKoelpin
Copy link
Contributor Author

I changed the function name to isNotNil.
If everything is fine for you l'd squash the WIP commits.

@LarsKoelpin LarsKoelpin force-pushed the master branch 2 times, most recently from 38065ee to 41cea32 Compare May 14, 2019 20:22
@LarsKoelpin LarsKoelpin changed the title Add isDefined Add isNotNil May 14, 2019
@Grohden
Copy link

Grohden commented May 27, 2019

Why not use ramda adjunct?
https://char0n.github.io/ramda-adjunct/2.18.0/RA.html#.isNotNil

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 isNotEmpty in the lib too.

@tommmyy
Copy link
Contributor

tommmyy commented May 27, 2019

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';
Copy link
Member

Choose a reason for hiding this comment

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

Missed in renaming.

* R.isDefined(null); //=> false
* R.isDefined(undefined); //=> false
* R.isDefined(0); //=> true
* R.isDefined([]); //=> true
Copy link
Member

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() {
Copy link
Member

Choose a reason for hiding this comment

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

Missed in renaming.

@LarsKoelpin LarsKoelpin force-pushed the master branch 2 times, most recently from 42a53b2 to a216b5c Compare June 5, 2019 18:22
@ben-eb
Copy link

ben-eb commented Jun 8, 2019

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.

@CrossEye CrossEye mentioned this pull request Jul 19, 2021
LarsKoelpin and others added 2 commits January 22, 2022 07:49
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.
@customcommander
Copy link
Member

@LarsKoelpin @CrossEye This pull request could not be merged because of a merge conflict with source/index.js and linter issues as it was predating the decision to add .js extensions to our import statements. That's the only thing I changed in this pull request, everything else is exactly as it was when you approved it @CrossEye.

If we still want this, this should now be ready to go.

@CrossEye CrossEye merged commit 4bb8dd8 into ramda:master Jan 22, 2022
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.

None yet

8 participants