pomack / thrift4go

Apache Thrift for the Go Language
129 stars 23 forks source link

Other solution for generated enums #57

Open GeertJohan opened 11 years ago

GeertJohan commented 11 years ago

New thread as reply on https://github.com/pomack/thrift4go/pull/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.

GeertJohan commented 11 years ago

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

GeertJohan commented 11 years ago

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

pomack commented 11 years ago

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

math.MinInt64 / 2

pomack commented 11 years ago

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

InvalidOrUnexpectedEnumValue

GeertJohan commented 11 years ago

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.

pomack commented 11 years ago

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

GeertJohan commented 11 years ago

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.

GeertJohan commented 11 years ago

https://github.com/thrift4go/enum-tryout/commit/e0606bc196daaea18cc49ee7f30f12eaf64b1021