I'm gonna answer all your comments in one go.
1) Interface vs abstract class: We had an interface and generics before. If we
were to use that, it would become something like this:
```
interface ConfigurationPersister {
void add(CacheElement config, CacheConfig existing)
...
}
```
and then in the `LocatorClusterManagementService`:
```
public void createCacheElement(CacheElement config) {
CacheConfig existingConfig = persistenceService.getConfig("cluster", cache);
ConfigurationPersister persister = (new
ConfigurationPersisterFactory()).generate(config)
persister.add(config)
}
```
See how we are using the config twice? Once to create the persister (since we
need to know what instance of the persister we need), and then passing it in to
persist it? That is clunky and weird in my opinion.
We could, of course, do this:
```
interface ConfigurationPersister {
void add()
...
}
```
and then in the `LocatorClusterManagementService`:
```
public void createCacheElement(CacheElement config) {
CacheConfig existingConfig = persistenceService.getConfig("cluster", cache);
ConfigurationPersister persister = (new
ConfigurationPersisterFactory()).generate(config)
persister.add()
}
```
This implicitly tells you that any ConfigurationPersister instance must store
the config it is initialized with -- and if you are doing that, since you can't
have variables on an interface, might as well use an abstract class.
2). Generics -- yes, we could add a generic to the `ConfigurationPersister`
class, and that would eliminate the need for some explicit downcasting we are
doing.
3). Not using newInstance()/storing the realizer instance -- could not figure
out a good way of using a Map<Class, Class> and not using newInstance().
Storing the realizer instance would not work nicely the way the code is written
right now, because the persister/realizer are instantiated with a given
existing config object/cache element -- if we were to store instances in the
map, we'd be potentially using stale state in our code. Of course, if the
method signatures took in the config itself, as described in point 1), this
wouldn't be an issue since you wouldn't have to store the config, but as
described in 1), that is awkward.
Hope I've answered your questions. For further discussions, would love to
connect again on Slack!
[ Full content available at: https://github.com/apache/geode/pull/3059 ]
This message was relayed via gitbox.apache.org for
[email protected]
