Other solution for generated enums #57

Open
GeertJohan opened this Issue Nov 18, 2012 · 8 comments

Comments

Projects
None yet
2 participants
Contributor

GeertJohan commented Nov 18, 2012

New thread as reply on #56

An other solution to this would be to create a package for each EnumType. However, I don't really like that solution as it will create a lot of packages and package references.

An third solution might be to define and instantiate a struct containing al EnumNames as fields.

Thrift code:

enum Status {
 One,
 Two,
}

Generated Go code:

package generated

// The Status enum type:
type StatusEnumType int64

// unexported struct for the values
type statusEnumStruct struct {
  One StatusEnumType
  Two StatusEnumType
}

// exported instance of the values struct
var StatusEnum = &statusEnumStruct{
  One: StatusEnumType(1),
  Two: StatusEnumType(2),
}

Usage example:

import (
    "fmt"
    "generated"
)

func main() {
    sOne := generated.StatusEnumType(1)
    if sOne == generated.StatusEnum.One {
        fmt.Println("equals")
    }
}

You could also add methods on the StatusEnum:

// method
func (se *StatusEnumStruct) FromInt64(i int64) {
    switch i {
    case 1:
        return se.One
    case 2:
        return se.Two
    }
}

// usage
sTwo := generated.Status.FromInt64(2)
// which is lots safer as:
sTwo := generated.StatusEnumType(2)
// because the FromInt64 method verifies the value to be existing.

Using the FromInt64 method might lead to the statusEnumType becoming unexported. Making it imposible to create a invalid StatusEnum value.

I think that this solution might be cleaner and safer, but will probably cost a lot more performance.

Contributor

GeertJohan commented Nov 18, 2012

I'm working on a more full featured example generated code now..

Contributor

GeertJohan commented Nov 18, 2012

I have uploaded some beginnings at the thrift4go organisation.
https://github.com/thrift4go/enum-tryout

The most interesting file:
https://github.com/thrift4go/enum-tryout/blob/master/gen-go/enums/foo.go

Owner

pomack commented Nov 18, 2012

Looks good, let's change unexpected to be something extremely unlikely:

math.MinInt64 / 2

Owner

pomack commented Nov 18, 2012

Make Unexpected a longer name though to ensure someone's enum name doesn't match:

InvalidOrUnexpectedEnumValue

Contributor

GeertJohan commented Nov 19, 2012

math.MinInt64 / 2
Why /2 ?
Although unlikely, there is still a change someone might pick this value for his/her enum value.
Should the generator then error on that? Or maybe check if the value is being used by the enum def in the .thrift file and if so, increment until a unique (non-used) value has been found?

InvalidOrUnexpectedEnumValue
Good catch, will do.

Owner

pomack commented Nov 19, 2012

Actually, Thrift defines all enums to be an i32 value. So math.MinInt64 is fine.

Contributor

GeertJohan commented Nov 19, 2012

Hmm... that would work yes..
Although we're kinda burning memory then...
I'm going to think about another solution for this, for now will change it to MinInt64.

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