alvaroloes / enumer

A Go tool to auto generate methods for your enums
Other
478 stars 111 forks source link

Make marshaling unknown values error #50

Open tv42 opened 5 years ago

tv42 commented 5 years ago

Hi. I'm using

type Action int

const (
    ActionFoo       Action = iota + 1
    ActionBar
)

to make the default zero value be an undefined value. I just noticed that some JSON marshaled as

{"action": "Action(0)", ...}

I would much prefer that marshaling enums that are not a valid value produce an error. It would expose my programming errors sooner. What do you think?

alvaroloes commented 5 years ago

I see the issue, but it would be a huge change in behavior right now for Enumer that would be backward incompatible. Take into account that Enumer marshals the value to JSON using the String() method that was already implemented in Stringer, which behaves like you are saying for unknown values.

I would say that the root issue is that Go allows you to store 0 in a variable of type Action, so Enumer assumes that it is right and serializes it.

However, there are cool solutions to your problem. For me, the best one is to define an undefined action with value 0.

type Action int

const (
    ActionUndefined Action = iota
    ActionFoo       
    ActionBar
)

That's it. Now the zero value is the action "undefined" and you can check that in your code.

Take into account that a variable of type Action can take any value, so I would recommend you to do the check by using the IsAAction method (it is autogenerated too):

var actionValid Action = ActionFoo
var actionInvalid Action = 1234 

actionValid.IsAAction() // returns true
actionInvalid.IsAAction() // returns false

I hope this helps.

tv42 commented 5 years ago

Stringer has no error return, so it shouldn't be refusing. It'd be trivial to change MarshalJSON to handle the undefined values, and only delegate to Stringer for the known good values.

You're trying to make me make the invalid value become a valid marshalable value; that is the opposite of my reason for introducing it.

toejough commented 4 years ago

In an ideal world, an enum "type" would only ever allow valid (passes IsAAction) values anywhere, but without 1st class enums of some kind in go, you'd have to use setters and do this manually everywhere.

toejough commented 4 years ago

I'd personally like to see @tv42 's request implemented. It is backwards incompatible, but who is actually intentionally depending on myAction: "Action(0)" being marshaled? I think the risk of introducing safer behavior here is low.

toejough commented 4 years ago

In any case, I do really appreciate the project as-is - it's already leaps and bounds better than raw ints!!