code review 135450044: go.tools/cmd/stringer: use const for names, slice for m...

go.tools/cmd/stringer: use const for names, slice for map strings
Improve the generated code by using a const instead of a var for the names string.
This requires some refactoring to get neat const() and var() blocks.
Also change the generate map code go use a single sliced string, to reduce the
size of the compiled representation (only one string value).

[r, 2014-09-04 22:24:03]

Hello gri (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.tools

[gri, 2014-09-04 22:49:18]

LGTM

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/golden_test.go
File cmd/stringer/golden_test.go (right):

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/golden_test.go#n...
cmd/stringer/golden_test.go:235: 2:  _Prime_map[0:2],
_Prime_names ?

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go
File cmd/stringer/stringer.go (right):

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:487: var indexes, names []string
in the past you insisted on 'indices' for the plural of index

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:512: // createIndexAndNameValue returns the pair of
declarations for the run. The caller will add "const" and "var".
createIndexAndNameDecl

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:532: return indexesVar, nameConst
return b.String(), nameConst

and get rid of indexesVar ?

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:544: g.Printf("const _%s_names%s   = %q\n", typeName,
suffix, names)
I'd print the const ... =

first, and then have the doubly nested for loop print all the strings to go
directly - no need for another buffer

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:549: g.Printf("\n")
move this down one line

[r, 2014-09-04 23:01:02]

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go
File cmd/stringer/stringer.go (right):

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:487: var indexes, names []string
On 2014/09/04 22:49:18, gri wrote:
> in the past you insisted on 'indices' for the plural of index

yeah, but it's not the word we're pluraling, it's the variable name. think of it
as "index"es.

but you raise a good point, and i've made all the generated variables singular
now. shorter and simpler.

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:512: // createIndexAndNameValue returns the pair of
declarations for the run. The caller will add "const" and "var".
On 2014/09/04 22:49:18, gri wrote:
> createIndexAndNameDecl

Done.

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:532: return indexesVar, nameConst
On 2014/09/04 22:49:17, gri wrote:
> return b.String(), nameConst
> 
> and get rid of indexesVar ?

Done.

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:544: g.Printf("const _%s_names%s   = %q\n", typeName,
suffix, names)
On 2014/09/04 22:49:18, gri wrote:
> I'd print the const ... =
> 
> first, and then have the doubly nested for loop print all the strings to go
> directly - no need for another buffer

done, and you're right, although it's uglier to print the quotes directly.

https://codereview.appspot.com/135450044/diff/1/cmd/stringer/stringer.go#newc...
cmd/stringer/stringer.go:549: g.Printf("\n")
On 2014/09/04 22:49:18, gri wrote:
> move this down one line

Done.

[r, 2014-09-04 23:06:53]

*** Submitted as
https://code.google.com/p/go/source/detail?r=5d786ea862b5&repo=tools ***

go.tools/cmd/stringer: use const for names, slice for map strings
Improve the generated code by using a const instead of a var for the names
string.
This requires some refactoring to get neat const() and var() blocks.
Also change the generate map code go use a single sliced string, to reduce the
size of the compiled representation (only one string value).

LGTM=gri
R=gri
CC=golang-codereviews
https://codereview.appspot.com/135450044

[gri, 2014-09-04 23:13:22]

FYI

https://codereview.appspot.com/135450044/diff/20001/cmd/stringer/golden_test.go
File cmd/stringer/golden_test.go (right):

https://codereview.appspot.com/135450044/diff/20001/cmd/stringer/golden_test....
cmd/stringer/golden_test.go:235: 2:  _Prime_map[0:2],
_Prime_name_ (?)