[infinispan-dev] Configuration visitor - Re: [JBoss JIRA] Commented: (ISPN-145) No transport and singleton store enabled should not be allowed

[Vladimir Blagojevic]

Hi Galder,

Although I agree with you that reflection is little bit harder to 
understand the advantage of this visitor pattern is many fold. Say, down 
the road, we have multiple of these visitors written, some of them, 
possibly, even written by third party clients. Our entire configuration 
tree class hierarchy is statically or compile time tied tied to these 
visitors and we don't want to go down this path. I'll give it another 
look, we can go static but we need to be aware of the trade offs.

If I understand you correctly the approach you are suggesting is not 
modular. It is generally considered not to be a good practice to tie 
logical processing to data structures themselves when the likelihood of 
extending logical processing is significant. The whole reason behind 
adopting visitor pattern on our configuration tree is the ability to add 
new operations to existing configuration object structures without 
modifying these structures. By using visitor pattern and adhering to the 
so called open/closed principle we allow room to do any kind of 
operations on our configuration object structures. It might seems that 
validation is the last thing we are going to do - but it is not, for 
sure. And tomorrow when there is yet another operation we need to 
complete on configuration tree the implementation will be a breeze.

Cheers,
Vladimir

On 09-09-10 4:11 AM, Galder Zamarreno wrote:
> Hi Vladimir,
>
> I had a look at the implementation and I'm not sure I understand the
> need for the reflection visit calls in AbstractConfigurationBeanVisitor.
> Using reflection makes harder to follow code and it's slower than typed
> calls and I'm not sure of the reason to use it here.
>
> Also, I don't see the need for a standard ConfigurationValidatingVisitor
> that does such validation. Instead, the way I see it working is
> SingletonStoreConfig having some kind of callback method being called,
> i.e. the traversalCompleted() call and within it, SingletonStoreConfig
> can, using the ComponentRegistry, retrieve the Transport component and
> see if it's set or not. Or alternatively, SingletonStoreConfig could use
> the passed InfinispanConfiguration to do its validation.
>
> To sum up, I think each AbstractNamedCacheConfigurationBean
> implementation should, if it requires to, have the ability to validate
> the configuration via some kind of callback. Such callback should
> probably is possibliy traversalCompleted() itself.
>
> WDYT?
>
>