Skip to content

add more computed macros#2219

Merged
stefanpenner merged 1 commit into
emberjs:masterfrom
tchak:computed-macros
Mar 26, 2013
Merged

add more computed macros#2219
stefanpenner merged 1 commit into
emberjs:masterfrom
tchak:computed-macros

Conversation

@tchak

@tchak tchak commented Mar 5, 2013

Copy link
Copy Markdown
Member
  • empty
  • notEmpty
  • not
  • bool
  • match
  • equal
  • gt
  • gte
  • lt
  • lte

@tchak

tchak commented Mar 5, 2013

Copy link
Copy Markdown
Member Author

@stefanpenner let me know if you think some are useless and if some are missing. I will add tests once we decided on the list of macros to add.

@stefanpenner

Copy link
Copy Markdown
Member

@tchak ember recent changed its empty -> isEmpty should the macro follow this pattern?

@tchak

tchak commented Mar 5, 2013

Copy link
Copy Markdown
Member Author

@stefanpenner I would prefer not to...

@stefanpenner

Copy link
Copy Markdown
Member

we already have: not empty bool

we may want or and any
or: returns the first truthy value
any: returns true if any value is truth, else false

@stefanpenner

Copy link
Copy Markdown
Member

and would be useful

@tchak

tchak commented Mar 5, 2013

Copy link
Copy Markdown
Member Author

or any and should take an arbitrary number of keys ?

@stefanpenner

Copy link
Copy Markdown
Member

yup

@stefanpenner

Copy link
Copy Markdown
Member
Em.computed.and = (dependentKeys)->
  dependentKeys = Ember.makeArray(dependentKeys)

  computed = (->
    properties = @getProperties.apply(@, dependentKeys)

    for name, value of properties
      return false unless value

    true
  )

  computed.property.apply(computed, dependentKeys)

Em.computed.or = (dependentKeys)->
  dependentKeys = Ember.makeArray(dependentKeys)

  computed = (->
    properties = @getProperties.apply(@, dependentKeys)

    for name, value of properties
      return true if value

    false
  )

  computed.property.apply(computed, dependentKeys)

Em.computed.map = (properties)->
  computed = (->
    for property, value of @getProperties(properties)
      if Ember.empty(value)
        null
      else
        value
  )

  computed.property.apply(computed, properties)

@tchak

tchak commented Mar 5, 2013

Copy link
Copy Markdown
Member Author

I am confused on the difference between or and any ? one of them should return the value where the other one juste a boolean?

@stefanpenner

Copy link
Copy Markdown
Member

@tchak yup

@devinus

devinus commented Mar 7, 2013

Copy link
Copy Markdown
Member

All of this seems...so familiar...

@stefanpenner

Copy link
Copy Markdown
Member

@devinus of what?

@devinus

devinus commented Mar 7, 2013

Copy link
Copy Markdown
Member

@stefanpenner Last time this came up.

@devinus

devinus commented Mar 7, 2013

Copy link
Copy Markdown
Member

And the time before that.

@devinus

devinus commented Mar 7, 2013

Copy link
Copy Markdown
Member

Also when it came up in SproutCore.

@devinus

devinus commented Mar 7, 2013

Copy link
Copy Markdown
Member

For the record, I love having common CP macros like this already available to me.

@chrisoverzero

Copy link
Copy Markdown

I also think that having these available would be useful. I was surprised to learn that computed properties can't be (for example) logically &&ed together:

showSummary: Ember.computed.bool('context.summary') && Ember.computed.bool('parentView.onIndex')

Having a clear, best alternative to what appears to be a sensible operation would be helpful.

@stefanpenner

Copy link
Copy Markdown
Member

@chrisoverzero

current possible:

hasSummary: Ember.computed.bool('context.summary')
parentViewHasOnIndex: Ember.computed.bool('parentView.onInded')
showSummary: Ember.computed.and('hasSummary', 'parentViewHasOnIndex')

But you present an interesting idea. What if CP's where more more chainable.

hasSummary = Ember.computed.bool('context.summary')
parentViewHasOnIndex = Ember.computed.bool('parentView.onInded')

showSummary: hasSummary.and(parentViewHasOnIndex)

It would be interesting to explore.

@stefanpenner

Copy link
Copy Markdown
Member

@tchak if you can add some tests for the new computed macro's I will merge these in.

@tchak

tchak commented Mar 25, 2013

Copy link
Copy Markdown
Member Author

@stefanpenner added some tests

stefanpenner added a commit that referenced this pull request Mar 26, 2013
@stefanpenner stefanpenner merged commit 2f07ae3 into emberjs:master Mar 26, 2013
@stefanpenner

Copy link
Copy Markdown
Member

BOOOM

@karlguillotte

Copy link
Copy Markdown
Contributor

Thank you @stefanpenner and @tchak. That is a nice addition to the API.

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.

5 participants