enumer icon indicating copy to clipboard operation
enumer copied to clipboard

suggestion: Use constant names rather than literals

Open seebs opened this issue 4 years ago • 5 comments

When using enumer, it's possible to end up with something where the only places that a constant would be directly used would be in the conversion to/from string, but enumer doesn't use the constant, it uses a literal value.

See examples here: https://github.com/dominikh/go-tools/issues/660

So for instance

var _DayNameToValueMap = map[string]Day{
	_DayName[0:6]:   0,
	_DayName[6:13]:  1,
	_DayName[13:22]: 2,
	_DayName[22:30]: 3,
	_DayName[30:36]: 4,
	_DayName[36:44]: 5,
	_DayName[44:50]: 6,
}

This could use Monday instead of the literal 0, and then it would be easier for analysis tools to detect that the constant value is in some way used. It might also be easier to read the resulting code, although I recognize that's not especially important with generated code.

seebs avatar Nov 23 '19 17:11 seebs

How about a compile-time check in the form of a non-executable function like stringer? It has the benefit of causing an "invalid array index" error during compilation if go generate is not run after the constants are reordered.

An example with day names:

func _() {
	// An "invalid array index" compiler error signifies that the constant values have changed.
	// Re-run the stringer command to generate them again.
	var x [1]struct{}
	_ = x[Monday-0]
	_ = x[Tuesday-1]
	_ = x[Wednesday-2]
	_ = x[Thursday-3]
	_ = x[Friday-4]
	_ = x[Saturday-5]
	_ = x[Sunday-6]
}

domsekotill avatar Feb 11 '20 12:02 domsekotill

@dmarkham do you want to open a PR here?

andig avatar Apr 27 '20 09:04 andig

@andig unfortunately my fork is no longer compatible, mainly because I have pulled in some of the PRs and ideas from here and went a slightly different direction. My PR is referenced here mainly to let people know I liked and used the idea, as a thank you.

dmarkham avatar Apr 27 '20 14:04 dmarkham

@dmarkham @alvaroloes is there any overarching plan if both packages serve a different purpose in the future or should both continue to be maintained?

andig avatar Apr 27 '20 14:04 andig

@dmarkham @alvaroloes is there any overarching plan if both packages serve a different purpose in the future or should both continue to be maintained?

Well in my mind these packages are just band-aids, waiting for real Enums in go. This package is widely used and stable. My package has more bells and whistles, that I wanted after researching what other forks, issues, and PRs that were getting submitted. They both are important. I just really wanted some of these features myself, and this package is just more conservative thus stable, and I think that's a good thing. I would rather move quickly, add the features, and I think that's a good thing also.

dmarkham avatar Apr 27 '20 15:04 dmarkham