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.
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.
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.
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
