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

eql and equal too similar words #21

Open
creynders opened this issue Feb 19, 2015 · 6 comments
Open

eql and equal too similar words #21

creynders opened this issue Feb 19, 2015 · 6 comments

Comments

@creynders
Copy link

Is there a specific reason you didn't want to use deepEqual or deep.equal?
All the other assertions are pretty readable, but I constantly need to double-check eql or equal. Also, IMO, it's a weird discrepancy in the API. Intuitively I'd think eql is an alias for equal, like gt et cetera.

@moll
Copy link
Owner

moll commented Feb 19, 2015

Hey!

Thanks for caring!

The inspiration for eql and equal came from a few sources: Ruby's RSpec, Should.js and most of all, the commonness of shorter equivalence operators to be less strict. Like in JavaScript: == vs ===. I believe tests should never do type-coercion therefore eql is still type-safe while being recursive.

Having said that, I get what your liking of the "deep" prefix. deep.equal would clash with Must.js's goal of decoupled matchers (sans the not link of course :), but I wouldn't mind deepEqual or deepEql to be aliases to all.

What do you think? Should deepEqual be merely a recursive version of equal (comparing object-identity otherwise) and deepEql then the same as eql now?

Just to clarify, the looseness of eql comes from the ability to compare value types like Date or your own types.

As Must.js hasn't hit v1.0.0 yet (but it should), we can still do a few breaking changes. And add a migration-helper sed script, of course. ^_^. Ideas, @koresar?

@creynders
Copy link
Author

Should deepEqual be merely a recursive version of equal (comparing object-identity otherwise) and deepEql then the same as eql now?

👍 Yeah that would be great!

@moll
Copy link
Owner

moll commented Feb 19, 2015

Would you agree that recursiveness should only apply to plain objects and arrays? The rest, in deepEqual's case, should be compared strictly (===)?

What, do you reckon, should eql then end up doing? Be a non-recursive version of loose checking? E.g:

{}.must.eql({}) // fail
[].must.eql([]) // fail
5..must.eql(5) // pass
new Date(2000, 5).must.eql(new Date(2000, 5)) // pass

@creynders
Copy link
Author

Good question.
Ok, when you start to think about this stuff, it suddenly gets pretty complicated. So, based on my gut feeling, I'd say equal is strict, as it is now. Two objects are deepEqual if they have strictly equal values for their members recursively AND a strictly equal prototype.
eql and deepEql reflect this, but with loose equality.

E.g.

9.must.equal("9") //fail
9.must.deepEqual("9") // fail
9.must.eql("9") // pass
9.must.deepEql("9") // pass

{}.must.equal({}) // fail
{}.must.deepEqual({}) // pass
{}.must.eql({}) // fail
{}.must.deepEql({}) // pass

{foo:"9"}.must.equal({foo:"9"}) // fail
{foo:"9"}.must.deepEqual({foo:"9"}) //pass
{foo:"9"}.must.eql({foo:"9"}) // fail
{foo:"9"}.must.deepEql({foo:"9"}) //pass

{foo:9}.must.equal({foo:"9"}) // fail
{foo:9}.must.deepEqual({foo:"9"}) //fail
{foo:9}.must.eql({foo:"9"}) // fail
{foo:9}.must.deepEql({foo:"9"}) //pass

o1 = new Foo("whatever");
o2 = new Foo("whatever");
o1.must.equal(o2) // fail
o1.must.deepEqual(o2) //pass
o1.must.eql(o2) // fail
o1.must.deepEql(o2) // pass

This is what seems most logical to me. However, it will be pretty nightmarish to migrate tests I think.

So, the easiest solution would be to alias eql with deepEqual and forget about deepEql.

@koresar
Copy link

koresar commented Feb 20, 2015

The naming of functions in the must.js is somewhat confusing (see #6). I agree with @creynders on that.

Must.prototype.equal(expected)

Assert object strict equality or identity (===).

I'd rename equal to strictEqual.

Must.prototype.eql(expected)

Assert object equality by content and if possible, recursively.

I'd rename eql to objectEqual.

@rightaway
Copy link

+1 for deepEqual to alias eql

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants