|
From: Daniel M. <mi...@pa...> - 2004-04-06 03:41:48
|
Message
> Daniel - thanks a lot your contribution! I made some *minor* code review
type polishing - be sure to test it to make sure I didn't break anything.
:-) Do you have unit tests developed you can contribute as well?
Thanks Keith. I'm glad to have my code edited. Unfortunately, I have not
developed any tests. I was hoping that someone could give me some pointers
on how to go about writing tests... I'm not very familiar with writing test
classes.
> I thought the code and design looked sharp. Good docs, too.
Thanks. I'm humbled by your compliments.
> Here were a few things that I noticed on initial review:
> - FieldChecks and JavascriptValidatorTag are a port over from the
commons-validator repository (FieldChecks is a particularly lengthy
copy-and-paste bit.) Have you tried delegating to the "real" commons-owned
versions of these and adding in the spring adaption stuff via decoration?
I know, they were copied pretty much straight from Struts. Can you elaborate
on "the real commons-owned versions" please? I was not aware that
commons-validator v1.0.2 had these things (I know v1.1.1 does have some of
this, but it is considered alpha). I would be glad to use different code
here. In my opinion, it is the weakest point of my contribution. However, I
thought using the code that has been (at least) "weather tested" in Struts
would be better than rolling my own. What exactly do you mean by
"decoration"? Can you give an example?
> - several setters must be called to set required
properties/dependencies post construction. This is much less of an issue
when using the container, but when used programatically it's more difficult
to enforce that the object in a valid, usable state. Fully-initializing
constructors make more sense to me in these cases (though I have to say I
appreciate you documenting those cases in the comments :-))
If you use the (undocumented--doh') constructors:
public ValidatorFactory(Resources[])
and
public BeanValidator(ValidatorFactory)
You need a ValidatorFactory to create a BeanValidator (or
NamedBeanValidator), but you shouldn't need to call any setter methods at
all if you use these constructors. I tried to provide options for both
setter and constructor setup, but maybe I overlooked something. Do enlighten
me if I have. I guess we need to modify the JavaDocs to document these.
> - The use of "beanName", when referring to the "formName" (which
typically corresponds to the bean class being validating) confused me a bit
(since when I hear beanName I think of a reference to a instance, not a
class.)
I guess I didn't provide very good documentation of this naming convention.
I used this naming convention because future versions of the
Commons-Validator will use "bean" instead of "form" to refer to the object
being validated (or at least it's on the TODO list).
> - The @TODO for per-request Locale support - could you expand upon
what you have in mind here? (do you have a need for working this in the near
future?)
I'm not sure what to do here--that's why it's a TODO. The commons-validator
can supposedly resolve validator definitions based on the locale of the
current request. However, since the current Spring Validator interface does
not have a locale argument, I think we are out of luck here. I implemented
it the way it is because I don't like to worry about locale except in my
view (I thought that's what messages.properties are for anyway). To conclude
this ramble: I am open to suggestions.
> A few other questions I had:
I'd be happy to answer any others you happen to think of as well :-)
> - How often do you use the NamedBeanValidator? When would you use it
(or in which cases is the BeanValidator that works with the class name not
good enough?
I do not see this as a highly-used class; it's just a bit more flexibility
for those extra special situations.
A NamedBeanValidator example is something that I need to add to the sample
app. I tried hard to explain it in the JavaDoc, but I guess I didn't do well
enough. I'll try again.
I envision people using NamedBeanValidator in two cases (there may be
others):
1. The class name of the bean being validated is not known or is the same
for multiple beans. In this case, the validator definition name can be
specified in the Spring configuration (but it is not mandatory, as it can be
specified at the time of validator invocation):
<bean name="myBeanValidator" class="...NamedBeanValidator">
...
<property name="beanName"><value>myBean</value></property>
...
</bean>
The following definition (in validation.xml) would validate this bean when
validate(command, errors) is invoked:
<form name="myBean">
...
</form>
If no bean name was specified in the configuration, the validator
invocation would look like this:
validate("myBean", command, errors);
Note that this invocation is not Spring-automatic.
2. A single bean needs to be (partially?) validated by different definitions
at different times. For example, on a WizardFormController, the validate
method would invoke
for page 1:
validate("formPage1", command, errors)
for page 2:
validate("formPage2", command, errors)
Each "formPageX" names a form definition in validation.xml that validates
the properties on that specific page. The final validation would simply
invoke each validate(...) method to ensure that all values entered during
the wizard flow are valid.
Hope that helped.
> - Anything else you have planned or on the wishlist for declarative
validation?
At the current time it meets all my needs. I am waiting for response from
the community. I am sure there will be plenty of wishes when people start to
use this thing in the wild.
Did you have any ideas or is there anything you are wishing for?
Can you see a better way to do something that I have already done?
Thanks,
Daniel
|