Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make addData(WithObject) *add* data (not overwrite) #75

Open
hypesystem opened this issue Jan 15, 2015 · 3 comments
Open

Make addData(WithObject) *add* data (not overwrite) #75

hypesystem opened this issue Jan 15, 2015 · 3 comments

Comments

@hypesystem
Copy link
Collaborator

It seems misleading that the reference to the data object is overwritten with whatever is given to addDataWithObject.

A more sensical way to do it, would be to merge the two objects, overwriting any values that were already present.

This would allow known values to be set at time of construction, and later values to be added after:

var message = new gcm.Message({
  data: {
    knownValue: "atConstructionTime",
    otherValue: "thatMayChangeBeforeSending"
  }
});
//State: { knownValue: "atConstructionTime", otherValue: "thatMayChangeBeforeSending" }

message.addData({
  newValue: "thatIsOnlyAddedLater"
});
//State: { knownValue: "atConstructionTime", otherValue: "thatMayChangeBeforeSending", newValue: "thatIsOnlyAddedLater" }

if(something) {
  message.addData({
    otherValue: "thatHasNowBeenChanged"
  });
  //State: { knownValue: "atConstructionTime", otherValue: "thatHasNowBeenChanged", newValue: "thatIsOnlyAddedLater" }
}
@hypesystem
Copy link
Collaborator Author

This would be a breaking change... maybe we should add a branch for breaking changes that will be merged into master at 1.0?

@hypesystem hypesystem added this to the v1 milestone Jan 15, 2015
@hypesystem
Copy link
Collaborator Author

it would make sense if there was also a set function doing pretty much what we do now, but for any value:

//Overwrite data field
message.set("data", {
    knownValue: "atSomePoint",
    otherValue: "theseOverwriteEverythingPreviouslySet"
});

//Set new value for one of the parameters sent to gcm
message.set("collapseKey", "newCollapseKey");

@hypesystem
Copy link
Collaborator Author

If data gets its own addData method, notification should have the same:

message.addNotificationField("title", "Hello, World");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants