enumer icon indicating copy to clipboard operation
enumer copied to clipboard

Make marshaling unknown values error

Open tv42 opened this issue 6 years ago • 5 comments

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?

tv42 avatar Jul 23 '19 21:07 tv42

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.

alvaroloes avatar Aug 07 '19 09:08 alvaroloes

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.

tv42 avatar Aug 07 '19 17:08 tv42

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 avatar Sep 18 '19 19:09 toejough

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 avatar Sep 18 '19 19:09 toejough

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

toejough avatar Sep 18 '19 19:09 toejough