@Param support for enum

[Aleksey Shipilev]

Hi Joe,

On 06/10/2014 12:02 AM, Joe Kearney wrote:
> Aleksey, as discussed here is the patch. It includes the main change in
> BenchmarkGenerator and StateObjectHandler, plus runtime and compile-time
> tests.

Thanks!

> Couple of notes:
> * I factored out addParameterValuesToGroup because the code was
> duplicated between step 1 and 2 of @Param discovery.

Looks good, but please move both static methods into BenchmarkUtils.

> * I'd advocate renaming getEnumConstants to getEnumConstantNames, but
> maybe that's just a matter of taste.

Truly, a matter of taste :)

> * I'm pretty sure there's a typo-bug in BenchmarkGenerator line 311, it
> should loop over fields from state#getFields, not over fields in clazz,
> an unused local variable warning.

D'oh! Thanks for catching it, can you submit a separate patch?

> Please let me know any feedback you have. I'm around tonight for any
> changes you'd like to make.

Okay, here it goes:

 * Static utility methods better be in BenchmarkUtils.
 * Please avoid long predicates: I understand it is easier to write out
" && !fi.getType().isEnum()", but I think these checks deserve a
separate inner if.
 * ...this, AFAIU, will save you from additional condition in
"isParamValueConforming": the control should not go there for blank
arguments

Otherwise looks good. I'll wait for your new patch.

Thanks,
-Aleksey.