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

Failing test for comparing two properties #3680

Closed
wants to merge 1 commit into from
Closed

Failing test for comparing two properties #3680

wants to merge 1 commit into from

Conversation

mehulkar
Copy link
Contributor

@mehulkar mehulkar commented Nov 2, 2013

It would be nice to be able to have a computed property for comparing two dependent keys.

@bradleypriest
Copy link
Member

There is https://github.com/jamesarosen/ember-cpm, but I agree, this would be nice in ember-core.
This is what I add to all my projects

Ember.computed.boundEqual = function(first, second) {
  return Ember.computed(first, second, function(key, value) {
    return get(this, first) === get(this, second);
  });
};

@stefanpenner
Copy link
Member

@bradleypriest a bound equal or similar, should likely enumerate equality checks across all arguments.

allEqual: Ember.computed.boundEquals('for','bar','baz')

I would assume one of the barriers to getting it in, is proper naming.

@mehulkar
Copy link
Contributor Author

mehulkar commented Nov 4, 2013

@stefanpenner, agreed, I couldn't think of a better name. Em.computed.equal seemed appropriate, but that's already used for something else. I can make a failing test cases for n keys as well, but I wasn't able to use registerComputed to make the tests pass.

@twokul
Copy link
Member

twokul commented Nov 4, 2013

It might sound a little radical but I would route for having a separate official CP library with CPs as boundEquals (to make core lighter).

@hjdivad
Copy link
Member

hjdivad commented Nov 4, 2013

@twokul this will be completely unnecessary if we get composable cps in. Then you just have Em.computed.get to distinguish bound from string values.

@twokul
Copy link
Member

twokul commented Nov 4, 2013

@hjdivad fair enough

@mehulkar
Copy link
Contributor Author

mehulkar commented Nov 4, 2013

@hjdivad what do you mean by composable CPs? Is there a discussion on this somewhere? It sounds awesome

@twokul
Copy link
Member

twokul commented Nov 4, 2013

@mehulkar we're working on them. We hope to submit a PR + open discourse discussion soon-ish.

@mehulkar
Copy link
Contributor Author

mehulkar commented Nov 4, 2013

Very nice. Let me know if I can be useful

@wagenet
Copy link
Member

wagenet commented Nov 9, 2013

I think we're going to work hard to get composable CPs in so I'm going to assume that it will be covered there. @mehulkar If you want to provide any review or feedback on that PR, that would be great. Thanks!

@wagenet wagenet closed this Nov 9, 2013
@mehulkar mehulkar deleted the eq-prop branch November 9, 2013 02:08
@mehulkar
Copy link
Contributor Author

mehulkar commented Nov 9, 2013

Will do, thanks @wagenet !

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

Successfully merging this pull request may close these issues.

6 participants