ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.31k stars 206 forks source link

Find a better default name for the `result` returned from send. #104

Closed hypesystem closed 8 years ago

hypesystem commented 9 years ago

This may look like a small issue, but suggesting code like this causes problems with readability:

sender.send(message, registrationIds, function(error, result) {

});

Why? Well, if we want to iterate over the internal results, we get this:

result.results.forEach(function(result) {
  //yep, that's right. two `result`s.
});

You may suggest simply changing the inner variable in the forEach to something else, but that won't change the fact that the most obvious name for an entry in a list called results is result.

What we can change, however, is the suggested name of the result-object returned from ´Sender#send`.


Alternatively, the result could be wrapped in some abstraction that would make it more accessible.

SarasArya commented 8 years ago

I think message_id would be fine. In case of success. In case of failure probably just a message

eladnava commented 8 years ago

@hypesystem maybe rename the result callback param in sender.send(msg, ids, function(error, result) to:

Or, maybe we can rename the internal results object to messages? Or devices? But we'd want to provide backward-compatibility.

hypesystem commented 8 years ago

I like response :smile:

eladnava commented 8 years ago

@hypesystem Sounds good -- so the modification should only be in the documentation, correct? And any tests as well.

hypesystem commented 8 years ago

Yes, that is correct.