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

Hi Galder, Mircea Ok I agree, lets make it static, not a problem. However, I do not agree that we should just make ValidationVisitor. I'll make a generic visitor and then a subclass ValidationVisitor.

There are many potential uses for visitor, I'll mention only the existing: implement applyOverrides in a much more meaningful, cleaner and faster algorithm and possibly inject as well. Cheers, Vladimir On 09-09-10 9:59 AM, Galder Zamarreno wrote: > > 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 >> > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev