On 8/13/07, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
>
> Igor and I are still in deadlock. We need a vote.
>
> I am arguing for removing final from setRequired so that I can do this
> in DateField:
>
> public FormComponent setRequired(boolean required)
> {
> super.setRequired(required);
> dateField.setRequired(required);
> return this;
> }
>
> That way you can just call setRequired on DateField and it will work.
> A caveat is that IF I want isRequired on the DateField to always work
> (this is not relevant for form processing, but maybe for things like
> adding behaviors etc), I should make isRequired final:
>
> public final boolean isRequired()
> {
> // prevent clients from overriding
> return super.isRequired();
> }
>
> Igor argues against this as he thinks the API hole is too dangerous.
it is necessary to always sync isRequired with setRequired(). it is the same
kind of contract as equals and hashcode. sure you can implement just equals,
and that will work. but you can later easily get in trouble because you did
not implement hashcode.
He was doing this:
>
> protected DateTextField newDateTextField(PropertyModel
> dateFieldModel)
> {
> return new DateTextField("date", dateFieldModel, new
> StyleDateConverter(true))
> {
> public boolean isRequired()
> {
> return DateField.this.isRequired();
> }
> };
> }
>
> This works when you override DateField's isRequired method (which I
> guess is a possibility), though the disadvantage of that is that you
> have to force your users this contract. So annonymous classes won't
> work and there is no way you can enforce this to clients (as you can't
> check that they did override the isRequired method appropriately). And
> things like
>
> return DateTextField.forShortStyle("date", dateFieldModel);
>
> won't work either.
notice that this is only an inconvinience for factory methods that generate
children inside formcomponentpanels. which has a much much much narrower
scope the formcomponent.setrequried nonfinal change.
A third alternative is to override e.g. onAttach:
>
> protected void onAttach()
> {
> super.onAttach();
> dateField.setRequired(isRequired());
> }
>
> this leaves a hole when there is a difference between the value of
> isRequired during onAttach and when rendering, though I guess this way
> we have no API changes at all.
you can also do this in onbeforerender().
-igor
We spent almost a whole day arguing back and forth about the proper
> meaning of checkRequired. We left that discussion in a deadlock, and I
> removed final from setRequired instead, as I believe it is more
> precise to override this method and distribute to children than doing
> this in onAttach, and it doesn't leave a hole (though that hole is
> arguably not very large) either.
>
> Can you please vote:
>
> [ ] please put final back to setRequired as I think the API hole is
> too big and doing it in onAttach or forcing sub components to override
> isRequired is good enough for me
> [ ] keep setRequired non final so that components can decide to do
> something useful when it is called, even though implementators have to
> think about what to do with isRequired
>
>
> Cheers,
>
> Eelco
>
