8:59 a.m.
On 09/10/2009 11:38 AM, Vladimir Blagojevic wrote:
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.
Hmmm, can you be more specific? We're talking about here in the scope of
Configuration validation.
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.
We've implemented the visitor pattern statically for invocation
execution (see org.infinispan.commands.Visitor. Mircea, you're the
original author so correct me if I'm wrong) and I don't we had any
problems dealing with extending it. In fact, I had to extend it myself
to cope with keySet and entrySet commands and wasn't that bigger deal
(https://jira.jboss.org/jira/browse/ISPN-94?page=com.atlassian.jira.plugin...)
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.
I see your point of not tying up the processing and the data structure.
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.
Hmmm, we could implement a swiss knife so that it can solve all the
problems in this world but before doing that, I'd like to hear more
specific examples ;). I mean, we could have a created a super generic
visitor pattern to deal with configuration validation, with
invocations...etc, but I'm against that since it's hard to read, hard to
follow and hard to debug. For reflection related nightmares, simply look
at JBC 1.x code.
So, to sum up, I see the point of using the visitor pattern but let's
time things a little bit. We're dealing with configuration validation
here, so let's stick that for the moment. Also, let's go for a
statically typed one similar to the one already used for dealing with
invocations. I'd imagine that applied to this use case, this would be
like: visitSingletonStoreConfig, visitTransportType...etc.
org.infinispan.config package:
ValidationVisitor {
void visitSingletonStoreConfig(SingletonStoreConfig ssc);
void visitTransportType(TransportType);
....
// instead of traversalCompleted, use more meaningfull name
validate()
}
SingletonStoreConfig:
public Object accept(Visitor visitor) throws Throwable {
return visitor.visitSingletonStoreConfig(this);
}
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?
>
>
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache