jagi / meteor-astronomy

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

Suggestion on getModified() #705

Open nathanschwarz opened 5 years ago

nathanschwarz commented 5 years ago

Hi,

I'd like to suggest a modification concerning getModifed().

I think getModified() should return a dictionary (hash map) instead of an Array. It would then enable us to make decisions based on the modified fields way more easily (calling external APIs, or throwing errors).

for example :

//model (not the complete definition)
{
      name: String,
      firstname: String
}

//setting firstname
doc.set('firstname', 'Arthur')

//getting modified value
const modified = doc.getModified() // { firstname: true, lastname: false }

if (modified.firstname || modified.lastname) {
   console.log("Hello there, I see you've changed your identity 🤓")
}

We could implement it by adding an optional field to the current API :

getModified({ toHashMap: true }) by converting the array to a hash map when necessary.

Or we could replace the current API.

In any case I find it way more practical to have a dictionary (which can be easily converted to an Array with the ES5 Object.keys() API)

lukejagodzinski commented 5 years ago

There is already such a feature implemented. The method name is getModifiedValues

On Thu, Jun 13, 2019, 10:29 PM Nathan Schwarz notifications@github.com wrote:

Hi,

I'd like to suggest a modification concerning getModifed().

I think getModified() should return a dictionary (hash map) instead of an Array. It would then enable us to make decisions based on the modified fields way more easily (calling external APIs, or throwing errors).

for example :

` //model (not the complete definition) { name: String, firstname: String }

//setting name doc.set('firstname', 'Arthur')

//getting modified value const modified = doc.getModified() // { firstname: true, lastname: false }

if (modified.firstname || modified.lastname) { console.log("Hello there, I see you've changed your identity 🤓") } `

We could implement it by adding an optional field to the current API :

getModified({ toHashMap: true }) by converting the array to a hash map when necessary.

Or we could replace the current API.

In any case I find it way more practical to have a dictionary (which can be easily converted to an Array with the ES5 Object.keys() API)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jagi/meteor-astronomy/issues/705?email_source=notifications&email_token=AAIAUXI3H6L6HWB3NRGRSLTP2KUY7A5CNFSM4HX6O6D2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GZNCB2Q, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIAUXKUSVXUTIOSNGPCJC3P2KUY7ANCNFSM4HX6O6DQ .