Skip to content

Commit

Permalink
Use ByteArray serialization to compare 2 class instances
Browse files Browse the repository at this point in the history
  • Loading branch information
@olivierphi
olivierphi committed Aug 22, 2012
1 parent f0d92ca commit db2c33c
Showing 1 changed file with 7 additions and 19 deletions.
26 changes: 7 additions & 19 deletions src/com/alanmacdougall/underscore/_.as
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
package com.alanmacdougall.underscore {
// imports
import flash.utils.ByteArray;
import flash.utils.Dictionary;
import flash.utils.Timer;
import flash.events.TimerEvent;
Expand Down Expand Up @@ -857,25 +858,12 @@ public var _:* = (function():Function {
}
}
} else {
// Objects with different constructors are not equivalent.
if ('constructor' in a != 'constructor' in b || a.constructor != b.constructor) return false;
// Deep compare objects.
for (var key:* in a) {
true;
if (_.has(a, key)) {
// Count the expected number of properties.
size++;
// Deep compare each member.
if (!(result = _.has(b, key) && eq(a[key], b[key], stack))) break;
}
}
// Ensure that both objects contain the same number of properties.
if (result) {
for (key in b) {
if (_.has(b, key) && !(size--)) break;
}
result = !size;
}
// Objects are compared via their serialized state.
var serializedA:ByteArray = new ByteArray();
serializedA.writeObject(a);
var serializedB:ByteArray = new ByteArray();
serializedB.writeObject(b);
return serializedA.toString() === serializedB.toString();
}
// Remove the first object from the stack of traversed objects.
stack.pop();
Expand Down

6 comments on commit db2c33c

@jklmli
Copy link
Collaborator

@jklmli jklmli commented on db2c33c Aug 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work. ByteArray serialization will strip out attributes tagged with the [Transient] metadata tag. If these attributes' values differ, the comparison will still claim they're the same.

@olivierphi
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right ; but do we necessary have to handle all the complexity of ActionScript 3 / Flex for a library which targets simplicity ?

Personnnaly I can live without a full-featured _.isEqual function, but in some cases it may be useful to have a more robust implementation, able to deal with these [Transient] metadata tag for example.

Do you want to try to write a cleaner _.isEqual AS3 function ?

@jklmli
Copy link
Collaborator

@jklmli jklmli commented on db2c33c Aug 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the implementation isn't robust, it should be removed. The original underscore.js targets both simplicity and robustness - see the rationale against a deep copy method here.

I've used [Transient] tags to prevent certain public attributes from being serialized, instead of writing a method to strip out all these (possibly-deeply-nested) attributes. _.isEqual would be broken on every instance of these classes.

@jklmli
Copy link
Collaborator

@jklmli jklmli commented on db2c33c Aug 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to mention...classes that use private/protected attributes can't be properly compared, e.g.

class Countdown {
  protected var count: int;
  public function Countdown(value: int) {
    this.count = value;
  }

  public function tick(): void {
    this.count -= 1;
  }

  public function get status(): String {
    return this.count == 0 ? "Done." : "Still ticking...";
  }
}

Unless there's some way to subvert access modifiers, I don't think it's possible to implement a bulletproof _.isEqual method in AS3.

@olivierphi
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we only can access public properties, and have to compare class instances with the informations we have access to.
The same problem occur in Javascript if you have "private" properties in functions :

var MyClass = function(param) {
  var private = param;
}
var instance1 = new MyClass(1)
  , instance2 = new MyClass(2);
// these 2 instances are not equal, but we can't know it

But this doesn't mean we can't use _.isEqual in Javascript :-)
Whatever language you use, all we have to know is that class instances can only be compared by comparing their pubic properties.

About the [Transient] tag : my personnal advice would be to add a isEqual() method on classes you want to compare which use this tag. This is a trick that the vanilla Underscore.js have - and Underscore.as have it too.

@jklmli
Copy link
Collaborator

@jklmli jklmli commented on db2c33c Sep 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

[Test]
public function testIsEqual(): void {
  var cat1: Object = {id: 1, eyeColor: 'brown'};
  var cat2: Object = {id: 2, eyeColor: 'brown'};
  var cat3: Object = {id: 3, eyeColor: 'black'};
  var cat4: Object = {id: 4, eyeColor: 'yellow'};

  var litter1: Object = {brown: [cat1, cat2], 'black': [cat3], 'yellow': [cat4]};
  var litter2: Object = {brown: [cat1, cat2], 'yellow': [cat4], 'black': [cat3]};

  Assert.assertTrue(_.isEqual(litter1, litter2));
}

This test will fail...I'm guessing ByteArray.writeObject serializes an object in the order in which its entries are defined, so comparing the toString()s of the two will show that they're different.

I think it's too risky to depend on ByteArray...there seem to be a lot of gotchas. _.isEqual should be removed until a robust implementation that does deep traversals and comparisons on built-in objects is written.

Please sign in to comment.