remove R.isEmpty

[davidchambers]

R.isEmpty is sugar for R.propEq('length', 0). The problem, though, is that people don't necessary realize that this is the case. Some might assume, for example, that R.isEmpty({}) evaluates true (#538, #1194). A colleague just ran into problems by assuming R.isEmpty(null) and R.isEmpty(undefined) evaluate true.

Sugar is nice, but in this case the desugaring is not obvious.

[buzzdecafe]

🐄

[CrossEye]

I know this one has had it's share of controversy. But I'm in favor of keeping it.

Saying "abc is sugar for xyz" is different from saying "abc can be implemented as xyz". I think this is the latter case. "pluck is sugar for map(prop)". I have no problem with that. It's simply noting that pluck is a syntactic simplification of the ideas expressed by map(prop).

But this case is different. The idea is expressed by isEmpty. One implementation of this is propEq('length', 0). But we have variously considered and/or adopted other variations of the same idea of isEmpty that could not have been expressed by propEq('length', 0). If we hadn't gotten bogged down by all the crazy details of what it might mean, this might have been our first function that operated on lists, strings, and plain objects. I could easily see taking that up again as in this case, it seems fairly reasonable (except for the pesky details!)

So I would rather keep the name. It is reasonably documented.

[paldepind]

👍 All the cases shows that people confuse the name of this function with what it does. It implies something else. R.propEq('length', 0) on the other hand is impossible to misunderstand.

We should not keep a confusing function just because we might one day implement something that does what it says. If that is the goal we should make that change now or remove it in the mean time to avoid harm.

[davidchambers]

The fact that the meaning of R.isEmpty has changed in the past and may change again in the future adds to my concern. There are clearly many semantically different ways to define a function named isEmpty in JavaScript. The only way to determine the behaviour with certainly is to read the source code (documentation is useful, of course, but is not guaranteed to be up to date and to cover every edge case).

[TheLudd]

With @CrossEye here.

I think the function is clearly documented. If it is considered a problem that people make incorrect assumptions about functions by just looking at the name then a whole lot of ramda functions are in trouble.

I don't think propEq is fitting. When using isEmpty I don't think "is the length property of the array equal to zero?", I think "does the array contain any elements?".

[svozza]

I'm with @CrossEye too. The documentation could not be clearer, if people want to make assumptions then that's their problem.

[CrossEye]

Mostly to me it's about expressive code. This:

var display = R.ifElse(R.isEmpty, R.always('no values'), R.join(', '));

demonstrates my intent much better than this:

var display = R.ifElse(R.propEq('length', 0), R.always('no values'), R.join(', '));

[kedashoe]

I think I might be the "some" @davidchambers is referring to as I just made this mistake :) And I do think something should be changed.

When using isEmpty I don't think "is the length property of the array equal to zero?", I think "does the array contain any elements?".

The thing is, I don't feel there is much (any?) precedence that isEmpty is just for arrays. Maybe someone who is more familiar with these can help out, but just checking a few other popular dynamically typed languages:

Ruby: http://stackoverflow.com/a/20677846/342588
Python: http://stackoverflow.com/questions/10545385/how-to-check-if-a-variable-is-empty-in-python (couldn't find a great resource)
Php: http://php.net/manual/en/types.comparisons.php

If I saw some code

x = [];
if (R.isEmpty(x))
...

, looks fine. But if I also saw

x = {};
if (R.isEmpty(x))
...

, I wouldn't think "what is this crazy person doing, isEmpty doesn't make any sense with an object". In python, all the "empty" stuff evaluates to false. In ruby you can use blank and in php you can use empty. Seems like if this function did that it would be very familiar to a lot of people. I'd like that, but maybe it is not for ramda. In which case creating a couple functions with more specific names or I guess just removing isEmpty is what I would vote for.

[scott-christopher]

Perhaps if isEmpty is going to exist, it should be defined in terms of empty?:

var isEmpty = R.converge(R.equals, R.empty, R.identity);

isEmpty([]); // true
isEmpty([1]); // false

It won't help with the {} case, but it would align the definitions of empty in Ramda and also add support for fantasy land monoids.

var Mul = function(x) { this.x = x };
Mul.prototype.empty = function() { return new Mul(1); }
Mul.prototype.concat = function(m) { return m.x * this.x };
Mul.prototype.equals = function(m) { return m.x === this.x };

var one = new Mul(1);
var two = new Mul(2);

isEmpty(one); // true
isEmpty(two); // false

[CrossEye]

In ruby you can use blank and in php you can use empty. Seems like if this function did that it would be very familiar to a lot of people. I'd like that, but maybe it is not for ramda.

I would have no problem approaching that again. We discussed this in #533, #538, and #539. In that last one, we realized just how confusing the notion was for objects and ended up dropping it. If we can find something we can live with, I think it would be quite reasonable to say we return true for these values alone: [], '', {}, null, undefined.

We might just have to live with (and document) the sort of weird cases such as the one I mentioned in #539:

var obj = Object.create(null);
Object.defineProperty(obj, 'foo', {
    enumerable: false,
    configurable: true,
    writable: true,
    value: 'bar'
});
obj; //=> {foo: 'bar'}
isEmpty(obj); //=> true  - Really?

[davidchambers]

Perhaps if isEmpty is going to exist, it should be defined in terms of empty?

Interesting idea!

[paldepind]

In that last one, we realized just how confusing the notion was for objects and ended up dropping it.

I don't find it confusing. Nonenumerable properties are nonenumerable exactly because they should not be part of such things.

@scott-christopher

Perhaps if isEmpty is going to exist, it should be defined in terms of empty?:

That is a splendid idea! Then we just need to fix empty so that it returns {} when passed and object. That would increase consistency since and object is a map and a map is a monoid with {} as its empty value.

[TheLudd]

@kedashoe

The thing is, I don't feel there is much (any?) precedence that isEmpty is just for arrays

I am not saying that

R.isEmpty({}) // => true
R.isEmpty({ foo: 'bar' }) // => false

would be a bad idea or incorrect. But thinking about it, I think I would prefer a function named hasProps or something similar to see if an object has any properties. This is more in line with what I understand as the ramda philosophy that a function does one thing and one thing only.

[CrossEye]

@paldepind:

In that last one, we realized just how confusing the notion was for objects and ended up dropping it.

I don't find it confusing. Nonenumerable properties are nonenumerable exactly because they should not be part of such things.

Well, I for one would still find this surprising:

typeof obj; //=> "object"
obj.foo; //=> "bar"
R.isEmpty(obj); //=> true

But the main objection was to this:

Empty values are null, undefined, "", array-likes with length 0, and objects with no enumerable own-properties.

This does not feel at all like the sort of API I want to document, maintain, and support. The last bit is the most problematic: "objects with no enumerable own-properties" sounds more like technical prestidigitation than clear definition. It might be the only thing really possible given the way the language works, but it's certainly not something to love.

[scott-christopher]

The last bit is the most problematic: "objects with no enumerable own-properties" sounds more like technical prestidigitation than clear definition.

An alternative way to state this might be in terms of R.keys, e.g. "an object with no keys, such that R.keys would result in an empty list.".

[CrossEye]

The last bit is the most problematic: "objects with no enumerable own-properties" sounds more like technical prestidigitation than clear definition.

An alternative way to state this might be in terms of R.keys, e.g. "an object with no keys, such that R.keys would result in an empty list.".

That's clearly better.

But I feel as though it's skirting the issue a bit. It feels as if there should be invariants that one could count on: if x is an array, then isEmpty(x) === true should imply

R.length(x); //=> 0
x[0]; //=> undefined
x[42]; //=> undefined

If x is a string, then it should imply

R.prop('length', x); //=> 0
R.nthChar(0, x); //=> ""
R.nthChar(42, x); //=> ""

And if x is an object then it should imply

R.length(R.keys(x)); //=> 0
x[""]; //=> undefined
x.foo; //=> undefined

But of course we can't get those final kinds of invariants for the object case, and it feels strange.

I actually do want to do this, even if we have to live with such ugliness, but I want us to go in with our eyes open to just how ugly this is. We should not sweep this under the rug. Most of Ramda's API is clean; a lot of it is elegant. This is neither.

[paldepind]

Well, I for one would still find this surprising:

I would certainly not. If foo is defined as an non-enumerable property then that was done exactly to get behavior like that. Would you be equally surprised that foo would not show up in Object.keys, not be included if you iterated over the keys or if you serialized the object to JSON? To me this sounds more like a general confusion about non-enumerable properties.

That's clearly better.

That is exactly the same just worded differently since Object.keys returns an array of a given object's own enumerable properties

[CrossEye]

Well, I for one would still find this surprising:

I would certainly not. If foo is defined as an non-enumerable property then that was done exactly to get behavior like that.

If I received an object from a distant part of the system, something not necessarily in my control, I have no idea that someone might have done this intentionally. I understand the language, and do know that such is possible, I've had little call to actually create non-enumerable properties.

I can easily imagine requirements that discuss what happens based on various combinations of the values of the foo, bar, baz, and quux properties of the object supplied, with lots of detailed rules that I might cut through at the beginning with an early escape in the form

if (isEmpty(obj)) {
  doSomething();
  return;
}

If someone passed in an object with a non-enumerable foo property here, I would be surprised. I would of course be able to rewrite to handle it, but I would be surprised.

That's clearly better.

That is exactly the same just worded differently

I was discussing the wording.

I want to know how to document such a function without making it sound like a laundry list of unrelated functionality.

[paldepind]

@CrossEye

If someone passed in an object with a non-enumerable foo property here, I would be surprised.

You cannot use misuse of non-enumerable properties as an argument against isEmpty. Sure, everybody can go ahead an cause confusion by making properties that should be enumerable non-enumerable. But that has nothing to do with isEmpty. It would cause confusion in lots of other Ramda functions and in general.

[CrossEye]

@paldepind:

I'm not trying to argue against this. I'm trying to make sure that we recognize that what we're stepping in. I want to do this. But I don't want us to do this, thinking it's actually simple. This is, in fact, going to be one of the ugliest APIs of all the functions in Ramda.

[paldepind]

I'm not trying to argue against this. I'm trying to make sure that we recognize that what we're stepping in.

I don't think where stepping in on anything. People have been able to misuse non-enumerable properties ever since ECMAScript 5. I don't see how this changes anything.

This is, in fact, going to be one of the ugliest APIs of all the functions in Ramda.

I disagree. This function is very simple. It compares anything that is a setoid and a monoid to its identity element. What is ugly about that?

I think this change recognized the bigger more generalized picture and is a step away from the very common overly specialized array-only functions.

[CrossEye]

This function is very simple. It compares anything that is a setoid and a monoid to its identity element.

I can define a setoid for the integers quite simply: two integers are defined as equal if they have the same remainder when divided by 5. So 12, 17, -23, and 4192 are all equal under this definition. I can easily make a monoid out of this, and in fact a full group with modular addition.

Or I can define a setoid for the complex numbers by defining them as equal if their real parts are equal. Again, it's easy to create a monoid from this.

In neither case can I be said to have clearly defined the setoid for the domain though. It's clear enough that I'm losing information that might be valuable. 17 and 4192 are in fact different numbers. 4.2 + 3.7i and 4.2 - 6.5i are not identical.

If our setoid for JS objects defines two objects equal, even though they have properties that differ (obj1.foo; //=> 'bar'. obj2.foo //=> 'baz'), then we are definitely losing information. The fact that these are non-enumerable properties and that maybe it's the best we can do, does not refute that fact that we're doing something rather uncomfortable.

For strings and arrays, there are clearly obvious setoid definitions. This is simply not true for Javascript objects.

(The issues we've had with merge suggest that there's probably also no obvious semigroup definition either; but that seems more likely tractable than this one.)

[TheLudd]

I think this change recognized the bigger more generalized picture and is a step away from the very common overly specialized array-only functions.

Is this a wanted direction to go in? Is there a link to a discussion about this? I prefer the highly specialized functions that ramda provide in opposite to more overloaded ones. Why not keep isEmpty as is and add hasProps for objects? One could even add hasEnumerableProps or something similair. Perhaps not the prettiest name but it leaves no doubt as to what the function does. And that to me is the core of ramda.

[paldepind]

@CrossEye

If our setoid for JS objects defines two objects equal, even though they have properties that differ (obj1.foo; //=> 'bar'. obj2.foo //=> 'baz'), then we are definitely losing information. The fact that these are non-enumerable properties and that maybe it's the best we can do, does not refute that fact that we're doing something rather uncomfortable.

I can also do this:

var a1 = [1,2,3];
var a2 = [1,2,3];
Object.defineProperty(a1, 'key', {
    enumerable: false,
    configurable: true,
    writable: true,
    value: 'val'
});
R.equals(a1, a2); //=> true

That also loses information. To me neither the object nor the array case are of any significance and I don't feel uncomfortable by any of them.

For strings and arrays, there is a clearly obvious setoid definition. This is simply not true for Javascript objects.

That is a matter of what one finds obvious. To me the definition is obvious.

@TheLudd

Is this a wanted direction to go in?

That is not my impression.

[CrossEye]

@TheLudd:

I think this change recognized the bigger more generalized picture and is a step away from the very common overly specialized array-only functions.

Is this a wanted direction to go in? Is there a link to a discussion about this? I prefer the highly specialized functions that ramda provide in opposite to more overloaded ones.

I think overall it's not. I agree that Ramda should continue to specialize in functions with simple APIs, usually taking single input types for any parameter. I had started to agree that this function was somewhat different. This discussion is making that harder to accept, but in the end I think it's still probably the best bet for this reason:

1. Ramda tends not to throw. It's meant to be a toolkit for grown-ups and doesn't hold your hand. If you give it bad data, it does the best it can with it.
2. The issues @davidchambers raised to start this thread are important. What should be the behavior, in light of (1), of isEmpty(null)? This is a predicate, so, since we don't want to throw, we have to return false or true, but false is extremely strange. "null is not empty?!!" So the only real choice, for null and undefined, is true.
3. Once we've expanded our input type a bit, it seems quite obvious to also expand it by both delegation and by covering plain objects. The delegation is simple enough. The object case faces the issues I've been discussing. But somehow, it still seems best to come as close as we can to the ideal and document it.

[CrossEye]

@paldepind:

I can also do this:

 [ ... define a non-enumerable property for an array ...]

That also loses information. To me neither the object nor the array case are of any significance and I don't feel uncomfortable by any of them.

Although we still haven't documented it as well as we should, Ramda's philosophy is that its data type is a list and if you pass it an array that does not represent a list, then anything might happen. we haven't so far tried to make any such claims about plain object.

For strings and arrays, there is a clearly obvious setoid definition. This is simply not true for Javascript objects.

That is a matter of what one finds obvious. To me the definition is obvious.

I really misspoke. There is a clear and obvious setoid definition for objects:

Two objects are equal if they have the same properties.

(We could get more formal about this if we really wanted, but I'm sure you understand what I mean.)

The trouble is that we have no way within the language to test this style of equality. It is because of this that we are stuck with the nasty "enumerable own-properties" formulation, or @scott-christopher's more palatable gloss on it.

[TheLudd]

@CrossEye I think I raised this in another issue but my expectation would be that if a function is not used according to its contract then the behavior should be undefined.

Now, one problem with isEmpty is the ambiguity of the documentation. The signature and the explanation speaks very clearly to me:

[a] -> Boolean
Reports whether the list has zero elements.

The examples seem to tell a different more complex story though. If the examples only showed isEmpty being called with an empty and a non empty array I would be totally fine with the behavior of isEmpty(null) being left unspecified.

There is the isNil function to determine if a value is null or undefined. I have sometimes found myself in the situation where my business specific code should do one thing if a value is an empty array, null or undefined and in these cases I have no problem using var isValid = R.either(R.isNil, R.isEmpty)

So to your points I say:

1. Ok, don't write code that throws for special cases, but also don't bend yourselves to not throw if the input is not what is expected.
2. In light of (1): leave the result undefined. If it throws can't read property length of undefined, so be it.
3. Now you are not down this path of needing to cover everything because you felt you needed to cover 2 things...

[CrossEye]

@TheLudd: I'm more than half-way convinced, based on all the controversy here. Ramda hasn't gone down this path often, and doing so here seems to be quite painful.

@buzzdecafe, @davidchambers, @raine: Any thoughts on this?

[davidchambers]

I prefer #1242, but if we decide not to merge that pull request I'd rather deprecate R.isEmpty than keep it in its current form.

[CrossEye]

I believe #1242 has the same API as is under discussion here. The implementation is definitely nice, but the questions here are more about what it should do. Though I've been wanting to make this change, I have had some serious qualms. The objections from @TheLudd have definitely resonated.

[buzzdecafe]

I'm with @TheLudd on this.

[davidchambers]

My preferred solution to the null/undefined issue is to add type checking throughout the library:

R.map(null);      // => TypeError: map requires a value of type Function as its first argument; received null
R.isEmpty(null);  // => TypeError: Cannot apply isEmpty to null

[scott-christopher]

If there is a concern regarding non-enumerable properties when passing objects to R.isEmpty, then this should be a deeper concern that extends to other functions like R.equals:

var obj1 = { a: 1 };
var obj2 = Object.create(null, {
  a: { value: 1, enumerable: true },
  b: { value: 2, enumerable: false }
});

R.equals(obj1, obj2); // true

Regarding passing undefined, or null to R.isEmpty, I would be happy with a type error being thrown for any type that we don't explicitly handle ourselves (arrays, objects, arguments, strings), or one that doesn't implement the necessary fantasy-land method(s).

[paldepind]

@CrossEye

The trouble is that we have no way within the language to test this style of equality.

That is not true. One can use Object.getOwnPropertyNames that returns both an objects enumerable and non-enumerable properties. It is supported in the same environments as those who support non-enumerable properties. But as I said before handling those in equals would be a mistake IMO.

[TheLudd]

I opened #1252 to show my suggestion on how to improve isEmpty.

[TheLudd]

@davidchambers

My preferred solution to the null/undefined issue is to add type checking throughout the library:

R.map(null);      // => TypeError: map requires a value of type Function as its first argument; received null
R.isEmpty(null);  // => TypeError: Cannot apply isEmpty to null

We have actually discussed this before in #481 and while I would like to have that functionality it can be difficult to achieve with good error messages like the ones in your examples. Because some functions are just composed from other functions. This was one of the problems I ran into when trying to work it out earlier. That being said, if ramda would like to go in this direction I would support and help to the best of my abilities.

[CrossEye]

@paldepind:

Two objects are equal if they have the same properties. [ ... ]
The trouble is that we have no way within the language to test this style of equality.

That is not true. One can use Object.getOwnPropertyNames that returns both an objects enumerable and non-enumerable properties.

Note the "Own" in that name. I suppose if we're operating in an environment which has Object.getProrotypeOf (as far as I can tell, that would include anything which would also let you create non-enumerable properties), we could build something which walks the prototype tree, collects ownPropertyNames along the way, and then uses that list. So it is testable. But it's damn ugly.

[davidchambers]

We have actually discussed this before in #481 and while I would like to have that functionality it can be difficult to achieve with good error messages like the ones in your examples. Because some functions are just composed from other functions.

I addressed this problem with help from @paldepind in sanctuary-js/sanctuary#50, and have since made improvements in sanctuary-js/sanctuary#61 and sanctuary-js/sanctuary#63. I'm convinced that Ramda should follow suit. @paldepind is on board. The next step is to convince @buzzdecafe and @CrossEye. ;)

I've been more productive since adding type checking to Sanctuary, in part because my colleagues stopped asking me to debug type mismatches in pipelines. This is a good thing because I don't particularly enjoy being a human type checker!

[davidchambers]

As a result of #1242 I'm no longer in favour of removing R.isEmpty.