FWIW, I'm NOT a fan of the static analysis option. Good design should be reflected in the code. We should not pick suboptimal abstractions (or no abstraction in this case) plus static analysis to validate odd design choices.
Gafy On Thu, Jan 4, 2024, 11:29 AM Piotr P. Karwasz <piotr.karw...@gmail.com> wrote: > Hi Ralph, > > On Thu, 4 Jan 2024 at 16:20, Ralph Goers <ralph.go...@dslextreme.com> > wrote: > > > > PropertyKey was created so that all “components” could be specified as > an enum value, thus ensuring consistency. > > > > The split between component and key is used in every declaration of a > property. > > > > It is unfortunate that the keys also had to be specified as static > constants so they could be used in @SetSystemProperty. I would have > preferred to be able to use the getSystemKey method. > > > > I do have objections. While the interface and enum usage isn’t perfect > moving back to static constants is much worse in my opinion. If I hadn’t > thought that I would have done that in the first place. > > > > If you can figure out a better approach just propose that but please > leave it the way it is until then. > > If consistency is the only requirement, we could use a static analysis > tool to check it. It should be rather easy to write an Error Prone > plugin that checks that: > > 1. The parameters of all calls to `PropertyEnvironment` methods are > public constant strings annotated with a specific annotation (e.g. > @Log4jPropertyKey), > 2. The strings are of the format "<component>.<property>" with a > limited number of possibilities for <component>. > > Remark that the list of components is not necessarily closed: > additional Maven modules or even third-party extensions could need to > define new component types. E.g. if we split > `DisruptorBlockingQueueFactory` into a separate Maven module, it will > still require an entry in `PropertyComponent` to have its own > properties. > > Piotr >
