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

Handle enumeration better #83

Closed
kernle32dll opened this issue Jan 1, 2018 · 4 comments
Closed

Handle enumeration better #83

kernle32dll opened this issue Jan 1, 2018 · 4 comments

Comments

@kernle32dll
Copy link
Contributor

kernle32dll commented Jan 1, 2018

A small annoyance that I found today. It revolves around enumeration, and how to work with them.

Take the following wsdl snippet:

<xs:simpleType name="country">
  <xs:restriction base="xs:string">
    <xs:enumeration value="DE"/>
    <xs:enumeration value="AT"/>
    <xs:enumeration value="CH"/>
    <xs:enumeration value="GB"/>
    <xs:enumeration value="ES"/>
    <xs:enumeration value="FR"/>
    <xs:enumeration value="SK"/>
    <xs:enumeration value="LU"/>
    <xs:enumeration value="PL"/>
    <xs:enumeration value="DK"/>
    <xs:enumeration value="IT"/>
    <xs:enumeration value="CZ"/>
    <xs:enumeration value="US"/>
    <xs:enumeration value="BE"/>
    <xs:enumeration value="NL"/>
    <xs:enumeration value="SE"/>
  </xs:restriction>
</xs:simpleType>

This gets translated to:

// Country was auto-generated from WSDL.
type country string

// Validate validates country.
func (v country) Validate() bool {
	for _, vv := range []string{
		"DE",
		"AT",
		"CH",
		"GB",
		"ES",
		"FR",
		"SK",
		"LU",
		"PL",
		"DK",
		"IT",
		"CZ",
		"US",
		"BE",
		"NL",
		"SE",
	} {
		if reflect.DeepEqual(v, vv) {
			return true
		}
	}
	return false
}

The problem is type country string - as the type starts with lowercase, you effectively cannot access that type outside the package the bindings are generated in (which is especially sour if you need that enumeration for requests).

Now, one could of course simple make it start with uppercase just like all the other types. However, that seems unsafe, as we already have a validate method. What I have in mind instead is something like NewCountry(input string), which would take a string (or the equivalent type of the enumeration), instantiate a new instance, run validate, and then return the instance or an error.

Edit: I just realize that this a bad idea, since this method would still return an unexported type - which is deemed bad style. So instead - how about title casing them like the other types, but also providing constants of sorts, so they could be used directly without instantiating?

Thoughts?

@fiorix
Copy link
Owner

fiorix commented Jan 2, 2018

I like your suggestion but it's probably better to keep the type exported (e.g. Country) to allow people to set values when they know what they're doing.

The problem of introducing new functions and types in the generated code is all the burden of insuring unique names etc.

@fiorix
Copy link
Owner

fiorix commented Jan 2, 2018

Also I'm surprised that this is being generated with unexpected symbol - might have been my mistake in the last patches when I changed trimns/goSymbol.

@kernle32dll
Copy link
Contributor Author

kernle32dll commented Jan 2, 2018

Okay, I will cook up a PR to make the enumerations start with uppercase, like the other types. Alright? @fiorix

We could still look at other options after #77.

Edit: NVM, I was working with an old branch. Enums are actually cased correctly in master!

@fiorix
Copy link
Owner

fiorix commented Jan 4, 2018

We can probably close this then.

@fiorix fiorix closed this as completed Sep 21, 2018
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

No branches or pull requests

2 participants