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

Alternate solution for handling the badge omitempty problem. #6

Closed

Conversation

justinrosenthal
Copy link
Contributor

This implementation makes Payload.Badge a *int instead of an int so omitempty can easily tell between empty (nil) and zero (0).

Inspiration taken from: https://willnorris.com/2014/05/go-rest-apis-and-pointers

Note: this is not a backward-compatible change!

This implementation makes Badge a *int instead of an int so omitempty
can easily tell between empty (nil) and zero (0).
@joekarl
Copy link
Owner

joekarl commented Jan 16, 2015

Yeah, this is definitely a viable way to handle this. This lib isn't the only one having to tackle that issue (see Coccodrillo/apns#22 and Coccodrillo/apns#23).
Over there they decided to just leave it the way it was and not break the interface.
I'm kind of with them in trying to avoid non-backward compatible interface changes.

As it stands right now, I'm ok with the current way of specifying badge < 0 for 0 and badge == 0 for omit.
That being said, the output json does need to be cleaned up so that badge: -1 doesn't get sent to Apple (perhaps an internal representation that uses a pointer?)

@justinrosenthal
Copy link
Contributor Author

Alrighty, it's your library 😁
Personally I think it's too easy for someone to accidentally screw it up as is... for instance:

func handleMarkRoomAsRead(roomID int) {
    previousCount := db.GetUnreadRoomCount()
    payload := &apns.Payload {
        Token: "some_token",
        Badge: (previousCount - 1),    // if previousCount == 1, whoops
    }
    apnsConn.SendChannel <- payload
}

@joekarl
Copy link
Owner

joekarl commented Jan 16, 2015

Okay, you've swayed me.

The PR looks good, give it a rebase and add a test to show that the serialization omits the badge key and another to show that Badge: Int(0) outputs badge 0 and I'll get it merged in.

Curious, I assume that you're using the lib? (or looking into using it)

@themartorana
Copy link
Contributor

You could also remove Badge and replace it with badge, which can be a *int and add functions SetBadgeNumber(int) and BadgeNumber() int.

This way you can call SetBadgeNumber(0) and have it work appropriately.

Sometimes when you have edge-cases to deal with in Go, albeit slightly non-idiomatic, I've found the idea of setters/getters works well.

@themartorana
Copy link
Contributor

Alternatively you could add a struct field SendBadge bool...

@joekarl
Copy link
Owner

joekarl commented Feb 14, 2015

Closed in favor of #9

@joekarl joekarl closed this Feb 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants