jagi / meteor-astronomy

Model layer for Meteor
https://atmospherejs.com/jagi/astronomy
MIT License
604 stars 67 forks source link

modified values #361

Open frankapimenta opened 8 years ago

frankapimenta commented 8 years ago

V1!

I was wondering...

when setting a new value it is added to modifiers. However if the value before was 'a' and the new value is 'a' it is added to modifiers anyways. Object is then considered modified (isModified() is true). What about implementing a fail quietly feature if before and after values are equal? Then isModified would be false. Does it make sense?

frankapimenta commented 8 years ago

There it goes a test in a function update defined in viewmodel from manuel:viewmodel. ........

console.log(editedPlayer.isModified()); editedPlayer.set('name', "name"); console.log(editedPlayer.isModified());

.. "false" "true" ..

console.log(editedPlayer.isModified()); editedPlayer.set('name', "name"); console.log(editedPlayer.getModified()); console.log(editedPlayer.isModified());

.. false false ..

console.log(editedPlayer.isModified()); editedPlayer.set('name', "name"); console.log(editedPlayer.getModified(true)); console.log(editedPlayer.isModified());

.. false false ..

is it suppose to be like this? When a log the object to the console and replicate it... the result is always in (false, true).

lukejagodzinski commented 8 years ago

I've just checked it and it works properly, so there is probably an error in your code.

frankapimenta commented 8 years ago

Hi.

I think it has to do with (this) context of viewmodel. I will take a look at that and come back.

frankapimenta commented 8 years ago

V1.

I was checking the code and it might be weird but:

Players list has one Player with name "whatever".

---1 let player = Players.findOne();

player.set('name', "whatever");

player.isModified() // true


---2 let player = Players.findOne(); player.set('name', "whatever"); player.getModified() // {} player.isModified() // false


is this working properly? player.isModified() should have been true no?

lukejagodzinski commented 8 years ago

Hmm how is it possible that the same code returns different results? Show entire code because the way you use it may influence document somehow.

lukejagodzinski commented 8 years ago

UPDATE: Oh in deed it's possible. Strange thing, it's probably a problem with clearing modifiers in some of the methods... I will fix it after releasing 2.0

frankapimenta commented 8 years ago

My questions to you might seem strange and weird but this is my day to day life in development 💨 I always find myself in these dark places 👻

I think that is in the clearing modifiers yes. I was going thru the code and I notice that:

lib/modules/fields/modified.js:8 -> what is unnecessary?

I don't have time to go thru your code more (unfortunately) but my suggestion (and I don't know if it is being already like this) would be to fail quietly if newValue == oldValue.

Thank you for your patient 👍

lukejagodzinski commented 8 years ago

With modifiers and setter there was always a problem because of a bad assumption at the beginning of development of Astronomy 1.0. I've entirely changed the way how it works right now and it's paying out with less bugs. There are a lot of things going on with setting values, getting modified values and getting modifiers and changing one little thing can influence some applications. So I have to be very careful about changes. It's necessary, it was an "ugly" fix to a problem somebody had :). The possible solution may be clearing modifiers also in the isModified() method. As I'm thinking about that, it should solve the problem. You can play with it if you want.

frankapimenta commented 8 years ago

I understand better now. I think it might solve the problem. It looks like you have a solution. 👍