2

Possible Duplicate:
is object empty?

 update: (id, data) ->
    toUpdate = @find(id)
    if toUpdate isnt {}
            console.log "hi mom"
            console.log toUpdate
            toUpdate.setProperty(key, value) for own key, value of data

    return toUpdate

 find:(id) ->
    result = record for record in @storage when record.id is id
    return result or {}

Given the following Mocha tests

describe '#update', ->
    it 'should return an updated record from a given id and data when the record exists', ->
      boogie = createData()
      archive = new Archive("Dog")
      dog = archive.create(boogie)
      result = archive.update(1, {name:"Chompie", age:1})
      result.name.should.eql "Chompie"
      result.age.should.eql 1
      result.emotion.should.eql dog.emotion

    it 'should return an updated record from a given id and data when the record does not exist', ->
      boogie = createData()
      archive = new Archive("Dog")
      dog = archive.create(boogie)
      result = archive.update(50, {name:"Chompie", age:1})
      result.should.not.exist

The result is

Archive #update should return an updated record from a given id and data when the record exists: hi mom
{ id: 1,
  validationStrategies: {},
  name: 'Boogie',
  age: 2,
  emotion: 'happy' }
  ✓ Archive #update should return an updated record from a given id and data when the record exists: 1ms
    Archive #update should return empty when the record does not exist: hi mom
{}
    ✖ 1 of 13 tests failed:
    1) Archive #update should return empty when the record does not exist:
    TypeError: Object #<Object> has no method 'setProperty'

...surprising, isnt it?

Community
  • 1
  • 1
  • Note, I ended up writing a simple library for Node to encapsulate this concern. If you don't wanna repeat implementing this yourself (test cases are included), check out https://npmjs.org/package/emptyObject – Visionary Software Solutions Sep 13 '12 at 15:36

1 Answers1

14

CoffeeScript's is (AKA ==) is just JavaScript's === and isnt (AKA !=) is just JavaScript's !==. So your condition:

if toUpdate isnt {}

will always be true since toUpdate and the object literal {} will never be the same object.

However, if @find could return a known "empty" object that was available in a constant, then you could use isnt:

EMPTY = {}

find: ->
    # ...
    EMPTY

and later:

if toUpdate isnt EMPTY
    #...

For example, consider this simple code:

a = { }
b = { }
console.log("a is b: #{a is b}")
console.log("a isnt b: #{a isnt b}")

That will give you this in your console:

a is b: false
a isnt b: true

But this:

class C
    EMPTY = { }
    find: -> EMPTY
    check: -> console.log("@find() == EMPTY: #{@find() == EMPTY}")

(new C).check()

will say:

@find() == EMPTY: true

Demo: http://jsfiddle.net/ambiguous/7JGdq/

So you need another way to check if toUpdate isn't empty. You could count the properties in toUpdate:

if (k for own k of toUpdate).length isnt 0

or you could use the special EMTPY constant approach outlined above. There are various other ways to check for an empty object, Ricardo Tomasi​ has suggested a few:

  1. Underscore offers _.isEmpty which is basically the for loop approach with some special case handling and a short circuit.
  2. Underscore also offers _.values so you could look at _(toUpdate).values().length. This calls map internally and that will be the native map function if available.
  3. You could even go through JSON using JSON.stringify(toUpdate) is '{}', this seems a bit fragile to me and rather round about.
  4. You could use Object.keys instead of the for loop: Object.keys(toUpdate).length isnt 0. keys isn't supported everywhere though but it will work with Node, up-to-date non-IE browsers, and IE9+.
  5. Sugar also has Object.isEmpty and jQuery has $.isEmptyObject.

A short-circuiting for loop appears to be the quickest way to check emptiness:

(obj) ->
    for k of toUpdate
        return true
    false

That assumes that you don't need own to avoid iterating over the wrong things. But given that this is just a test suite and that an emptiness test almost certainly won't be a bottle neck in your code, I'd go with whichever of Underscore, Sugar, or jQuery you have (if you need portability and have to deal with the usual browser nonsense), Object.keys(x).length if you know it will be available, and (k for own k of toUpdate).length if you don't have the libraries and have to deal with browser nonsense and aren't certain that toUpdate will be a simple object.

Community
  • 1
  • 1
mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • So it checks explicitly for the same reference, not just the same type and value? What's an idiomatic way of checking for empty object...ah, well played. This feels almost evil, but rather delicious `toUpdate.setProperty(key, value) for own key, value of data when ((k for own k of toUpdate).length isnt 0)` – Visionary Software Solutions May 07 '12 at 06:52
  • Would it be "broken" to add `isEmpty()` to `Object.prototype`? I know purists get fussy about messing with the prototype, but it seems really egregious that's not there. Is there a reason not to add it? – Visionary Software Solutions May 07 '12 at 06:58
  • @VisionarySoftwareSolutions: I'm not exactly a purist but monkey patching core objects smells bad to me, especially for a generic name like `isEmpty`. You could just grab Underscore and use [`_(toUpdate).isEmpty()`](http://documentcloud.github.com/underscore/#isEmpty), Underscore is pretty handy to have around anyway. I'd call that `toUpdate.setProperty(...)` construct rather confusing, a two-liner would be better. – mu is too short May 07 '12 at 07:04
  • @muistooshort @VisionarySoftwareSolutions if you're on an environment that supports `Object.defineProperty` there is nothing wrong with extending native prototypes carefully. – Ricardo Tomasi May 07 '12 at 21:18
  • @RicardoTomasi: My main problem is the potential for conflicting monkey patches so I avoid it unless I'm adding something that should already be there or I prefix the names to avoid conflicts; I suppose a lot of that falls under *carefully* though. And `defineProperty` needs IE9+, doesn't it? – mu is too short May 07 '12 at 21:33
  • 1
    A couple other ways to check, depending on support: `if JSON.stringify(obj) is '{}'`, `if Object.keys(x).length` – Ricardo Tomasi May 07 '12 at 21:34
  • @muistooshort yes, it does, but not everyone is coding for a browser (or all browsers) - this looks like a node app. – Ricardo Tomasi May 07 '12 at 21:35
  • @RicardoTomasi: How portable is `keys` though? The inlined loop is just a quick'n'dirty inlined `keys`. – mu is too short May 07 '12 at 21:37
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/10989/discussion-between-ricardo-tomasi-and-mu-is-too-short) – Ricardo Tomasi May 07 '12 at 21:38
  • I'm actually really interested to see the results in terms of the portability question for Ricardo's proposed snippets as other "answers" to this question, recorded for posterity. Could you add discussion of JSON.stringify() and Object.keys(x).length? – Visionary Software Solutions May 07 '12 at 22:37
  • @VisionarySoftwareSolutions: I added some notes and options based on my discussion with Ricardo. – mu is too short May 08 '12 at 03:01