When comparing two arrays, rather than comparing numeric indexes, maybe try to compare the content ?

Yup, just on a whim. :)

Occur on instantiate:
$snapshot = new ObjectSnapshot($object);

According to me in ObjectSnapshot ligne 48, $object is missing.
$this->data[$reflProperty->getName()] = $reflProperty->getValue($object);

As the collection transforms itself into an association array (primary_key => object), once the Set has been calculated, it should revert back to a collection material (numeric_key => object).

Currently, if there is an object in the properties of an object, only the sha1 will be compared (meaning that only a new object instead of the old one, not the modified one, will be compared to the old one).

The problem lies within the fact that the object is not copied, but is a reference since php5 :

<?php

class Foo {
     public $bar = 'baz';
}

$foo = new Foo;

$bar = $foo;

$foo->bar = 'fubar';

var_dump($foo->bar, $bar->bar); // will dump fubar fubar

But I also can't clone the object if it is an object, because the sha1 will differ from one clone to another, considering it to be a whole new object altogether.

I think I should check if i should, in the data generated by the snapshots, store the value into a value object setting also the type, transforming it into something I may understand... (like an object snapshot, or whatever).

This is more sexy than the PSR-0. And also more maintainable, but that is another point. :)

If we try to compare a ArraySnapshot or ObjectSnapshot with a null key in the new key, it bugs with a bad argument exception on AbstractSnapshot::isComparable.

I should restore the previous behaviour which checked the new and old keys in the Set class.

Comme décrit ici, lorsqu'on remplace les id par des lettres, on a une 500 au lieu d'une 404.
A moins que ça soit le standard django?
Quoi qu'il arrive, bug ou non, c'est pas prioritaire.

Which wold be used to snapshot data with a notion of priority. This would remove the call to any normalize in built-in snapshots and thus remove this method for the AbstractSnapshot.

As this may be a BC Break, I'm tagging this for 2.0.

To compare values between two states, an appropriate tool to replace hard coded comparision could be sebastian bergmann's comparator

Instead of only see what has changed, i should soften up the Set to also accept different array sizes when computing the changeset, so it shows what has added and removed.

Just opening an issue so that I can check it up later. Any comments on how I should proceed are welcome.

As Symfony's ProperyAccess was integrated with the CollectionSnapshot (cf #22), and as in a collection the whole objets should look alike (collection of objects, of arrays, ...), we should not try to modify the primary parameter.

If the snapshot is used with a collection of arrays, then we should either have a PropertyPath directly fed to the pkey argument, or a valid property path either way [key]. If it is a collection of objects, then either a PropertyPath object, either a key path.

This should be way more flexible than it actually is right now, instead of doing a wild guess depending on the types of each elements fed into the collection, which could be wrong.

The raw data which is set in the AbstractSnapshot can be wrong.

Example : on objects, it may return the new state (references, etc) instead of the state it was when it was snapshotted.

I added this property in order to be able to act on the real data if I ever needed it, like on the diff. I should try to find another way to do that, without really relying on the "raw state".

I'm not a huge fan to inject a setClass into the AbstractSnapshot. I think it should be a SetInterface object, as an DIC would like to have.

Just writing this issue to think about that later.

As the PropertyPath is already required due to Collections, it could be useful to have the property path for each property changes

Allow to use a different Set (an extension, or something like that) in the built-in snapshots rather than only the built-in Set.

Currently, if you want to use something else, you have to reimplement the diff method of AbstractSnapshot... Even if all it does is just changing the class used.

Because 5.4 is now EOL for a while. Should bump to 5.6 (.. or 7 ?). As we are trying to stick close to [http://semver.org](Semantic Versionning), this should be considered on next major bump ("version 2.0")

To allow to customize even further the other classes that could need some of the AbstractChange capabilities... Without extending it.

When a snapshot is normalizing its data (via a call to AbstractSnapshot::normalize()), as new object & array snapshots are made, the setClass parameter is not propagated to these childs.