[GitHub] [incubator-druid] jihoonson opened a new issue #8328: [DISCUSS] Code generation for commonly implemented basic methods

[GitBox <gi...@apache.org>]

jihoonson opened a new issue #8328: [DISCUSS] Code generation for commonly implemented basic methods
URL: https://github.com/apache/incubator-druid/issues/8328
 
 
   I recently found a bug in `ArrayOfDoublesSketchAggregatorFactory .equals()` (fixed in https://github.com/apache/incubator-druid/pull/8326) which made me think that we may need a better way to implement and maintain them. `equals()`, `hashCode()` and `toString()` are very commonly used functions and not easy to catch a mistake if ppl write them manually. To avoid mistakes, I usually use the code generation function of Intellij. This function is good but is not enough to automatically maintain those implementations on code changes.
   
   The [Lombok](https://projectlombok.org/) is a library to address this kind of use case. It can automatically generate codes of `equals()`, `hashCode()`, and `toString()` at compile time so that we can expect the implementations are always up to date. Also, they are programmatically generated and it's safer than human codes.
   
   I also find the Lombok library useful when the [builder pattern](https://en.wikipedia.org/wiki/Builder_pattern) is useful. It provides an annotation called `@Builder` which automatically generates a builder implementation for a class. This could be very useful to maintain the test codes for user-facing configuration classes. For example, I have added a couple of new configurations to `ParallelIndexTuningConfig` which also required to fix all places calling the constructor of the class. With the builder, I think we can reduce the required changes which could reduce the PR size.
   
   The downside of using a library such as Lombok is the build time might be longer since it needs to generate more source codes. But I still it would be worth to take it.
   
   If we go with this way, I think we can start with `equals()`, `hashCode()` and `toString()` methods and gradually change those implementations with Lombok annotations (like 10 classes in each PR). Probably we need unit tests as well if those methods of a class only take into account of only a subset of variables defined in that class.
   
   Welcome any thoughts.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org