isEmptyString & isNonEmptyString about ramda-adjunct HOT 21 CLOSED

@tycho01 need some collaboration over this. Does this even make sense ? Thought I have one production use case in my current project.

const description = 'user input';
if (isNotNull(description) && isNotEmptyString(description)) { return description }

from ramda-adjunct.

Sounds like just if (description) covers this use-case? That said, I think this whole function-based approach primarily shines if you're doing composition/map. That is, without need for a function one could technically be shorter off just writing description != '' for this.

But yeah, why not? :)

So I've personally tended toward higher abstraction, and wanted a slightly different truthy/falsy check for arrays/objects, here's what I've been using for that.

export let falsy: (v: any) => boolean = R.either(R.isEmpty, R.not);
export let truthy: (v: any) => boolean = R.complement(falsy); // differs from JS's 'truthy': []/{} -> false.

For other cases, in the case one would prefer higher abstraction, where you want a function version of the built-in truthy/falsy check, one could also use the built-in Boolean (/ R.identity) or R.not for positive/negative checks respectively.

from ramda-adjunct.

Sounds like just if (description) covers this use-case? That said, I think this whole function-based
approach primarily shines if you're doing composition/map. That is, without need for a function one
could technically be shorter off just writing description != '' for this.

if (description) would cover the usecase, but we have a airbnb codestyle where everything should be checked explicitly except for booleans. As for imperative over declarative system here, I like the first version better, I can read it faster and I know instantly what is going on there.

if (isNotNull(description) && isNotEmptyString(description)) { return description }`
if (description !== null && description !== '') { return description }

Also I like you falsy, truhy approach ;]

from ramda-adjunct.

Yeah. I wish the built-in checks used this logic for truthy/falsy; it could make things a bit more terse when checking objects/arrays...

from ramda-adjunct.

Back to the original problem...it seems that isNotEmptyString doesn't make sense alone.

if (isString(description) && isNotEmptyString(description)) { return description }`

What makes more sense is

isNotEmptyString = both(isString, complement(isEmptyString));

from ramda-adjunct.

allPass could be both for just two, for complement(isEmptyString) just Boolean could do, but yeah, whatevers. With composition like that you'll manage to avoid boilerplate, yeah.
P.S. if it includes isString I'd name it isNonEmptyString. :)

from ramda-adjunct.

isEmptyString = eq('');
isNonEmptyString = both(isString, complement(isEmptyString));

isNonEmptyString(null) -> false
isNonEmptyString('test') -> true
isNonEmptyString(udefined) -> false
isNonEmptyString('') -> false

Ok I guess it make sense now, thanks. Renamed to isNonEmptyString to distinguish that this is not just a simple complement.

from ramda-adjunct.

Conclusion: Implement two new type functions

// isEmptyString :: * -> Boolean
export const isEmptyString = equals('');

// isNonEmptyString :: * -> Boolean
export const isNonEmptyString = both(isString, complement(isEmptyString));

@tycho01 thanks for all your input.

from ramda-adjunct.

Sure. :)

from ramda-adjunct.

@guillaumearm wanna take this one also ?

from ramda-adjunct.

Absolutely :-D
@char0n : I think you can assign this issue to v2.4.0, sorry for the late response, hard week for me
:-P

from ramda-adjunct.

I'm on it ;-)

from ramda-adjunct.

I have a problem with the proposed isEmptyString implementation above :

// isEmptyString :: * -> Boolean
export const isEmptyString = equals('');

@char0n : Should isEmptyString(new String('')) return true ?

Edit:
In fact, to be consistent with isNonEmptyString behavior, maybe should we use this implementations :

// isEmptyString :: * -> Boolean
export const isEmptyString = both(isString, isEmpty);

// isNonEmptyString :: * -> Boolean
export const isNonEmptyString = both(isString, isNotEmpty);

from ramda-adjunct.

IMHO it should return false for isEmptyString(new String('')). It is the case that in JavaScript '' !== new String('') so I would not worry about handling this usecase. They're just not the same.

Regarding the composition, I understand the consistency argument, but again in JavaScript it is case that '' === '' but {} !== {} and [] !== []. I don't think complex composition like both(isString, isEmpty) is needed for this usecase and simple equation should do. It is simple and quick, and I just like these two attributes. We should always strive to use the simplest workable solution.

If you look at the source of isEmpty, it calls empty on argument (monoid) and then doing the comparison using the equal...so basically it is doing the equals('', arg) which is what I am proposing.

from ramda-adjunct.

OK, it sounds good, but i have some trouble in isNonEmptyString tests because of isEmpty :

> R.isEmpty(new String('abc')) // => false
> R.isEmpty(new String()) // => false

so there is a problem here :

eq(RA.isNonEmptyString(new String('abc')), false); // test throw

from ramda-adjunct.

We can get around using a kind of isPlainString internal function in isNonEmptyString implementation.

const isPlainString = x => typeof x === 'string'

from ramda-adjunct.

What about something like this one ?

const isNonEmptyString = R.allPass([RA.isString, RA.isNotObj, RA.isNotEmpty]);

from ramda-adjunct.

I don't know.
Semantically speaking, I prefer your solution.

But according to you, which one is fastest ?

const isNonEmptyString = R.allPass([RA.isString, RA.isNotObj, RA.isNotEmpty]);

or

const isPlainString = x => typeof x === 'string';
const isNonEmptyString = both(isPlainString, isNotEmpty);

Need benchmark ?

from ramda-adjunct.

I'd prefer also the first one even though it is going to be slower. In the context of Equational Reasoning I prefer to compose new parts from the parts that are already available rather than create new add hoc parts just to satisfy the speed property. My main argument in previous comment was not about speed it self but the simplicity. Speed is secondary in this case. both(isString, isNotEmpty) can be translated directly into equals('') so we are basically creating a fushion showcut here because we understand the behavior of both and we know that they equal.

from ramda-adjunct.

Ok, so I go for the first solution, did you want I create 2 PRs ?

from ramda-adjunct.

yup thanks

from ramda-adjunct.