Thats all fine, but it was not build/designed that way period...
The biggest api break was at the moment we removed final, which was
there for quite sometime, and then sneaky add javadoc that it is
required to call isRequired first before calling this method! but
before that for this public callable method this was not needed, so
everybody who uses that method suddenly breaks because of a sneaky
javadoc update and the removal of the final keyword.
What the javadoc should have stated is that overrides should ALWAYS
first check the isRequired() itself just like the method that they
override already did for 2 years!
The removal of the final keyword had in this particular case big
consequentes it really changed the api behavior.
Johan
On 3/20/08, Vitaly Tsaplin <[EMAIL PROTECTED]> wrote:
> What if a requirement cannot be met if an input contains only
> spaces. I would override the checkRequired to do so. And it could be
> done for a text field. Why not?
>
> Anyway if I can override checkRequired I may not be calling
> isRequired and it's perfectly legal. Because the method is under my
> control. But...setRequired is still there and anyone can call it
> excpecting it's normal behavior. Setting required property to false
> (setRequired) should prevent any requirement check whatever it would
> be.
>
> On Thu, Mar 20, 2008 at 12:57 AM, Johan Compagner <[EMAIL PROTECTED]>
> wrote:
> > hmm
> >
> > the more i think about it
> > the more i stand with my initial reply.
> >
> > checkRequired was always meant to be standalone
> > it always checked from day 1 if required must be checked (thats why it is
> > called *check*Required) and then it actually did the test.
> > it was first protected final and then a long time public final
> >
> > validateRequired does something else
> > it calls checkRequired and with that return value it sets an adds an
> error
> > message
> > But that is not always what you want (the reason why checkRequired is
> > public)
> > so validateRequired is not a substitute and i guess checkRequired has to
> be
> > public because we made it public for a reason. (see the thread)
> >
> > The only thing i come up with now is, but that will break all
> > formcomponentpanels that did have implemented checkRequired(),
> > that we make it final again (keep it public) and create a protected
> > overridable method that can be used in FCP
> > something like
> >
> > protected boolean doRequiredInputCheck()
> > {
> > }
> >
> > or what ever better name we can come up with.
> >
> > that way we suddenly dont have a completely different definition of a
> > method
> >
> > johan
> >
> >
> >
> >
> >
> > On Thu, Mar 20, 2008 at 12:03 AM, Johan Compagner <[EMAIL PROTECTED]>
> >
> >
> > wrote:
> >
> > > no some protected methods can be called just fine from the outside
> world
> > > like validateRequired() (that could be public yes)
> > >
> > > But checkRequired() doesn't make much to call from the outside world
> > > because we have validateRequired()
> > > except that validateRequired() does set an error then and doesn't
> return
> > > the actual boolean
> > > So i guess that that is the reason why checkRequired() is public.
> > >
> > > it was first public final that as removed for FormComponentPanel
> > > and when it was public final it made perfect sense to also check for
> > > required there..
> > > So the definition is really changed when we removed final before that
> it
> > > was really what i thought initially because it is a public callable
> method
> > > (back then the javadoc also didnt specify that it was typically
> required
> > > that isRequired must be callled)
> > >
> > > so suddenly final is gone and the definition changed...
> > > Nobody would call isRequired() before it would call checkRequired()
> > >
> > > and it was it first few months (2006) it was protected final,
> > > then you made it public final because of this thread "[Wicket-user]
> > > Components Label for FeedbackMessage"
> > >
> > > So if anybody programmed against that method back then because of that
> > > thread. Those are not calling isRequired() first before calling
> > > checkRequired()
> > > Even if i type this i still find it odd that i have to do that first,
> > > looking at the names...., if i read formcomponent.checkRequired() then
> i
> > > will always asume that it
> > > will not return the wrong value if isRequired == false.. maybe it is my
> > > bad englisch but that will not change for me)
> > >
> > > johan
> > >
> > >
> > > On Wed, Mar 19, 2008 at 11:40 PM, Igor Vaynberg
> <[EMAIL PROTECTED]>
> > > wrote:
> > >
> > > > do we need to add that to all our protected methods? if someone needs
> > > > it, and they probably do since we made it public, we should prob make
> > > > validateRequired() public - it is final iirc
> > > >
> > > > -igor
> > > >
> > > >
> > > > On Wed, Mar 19, 2008 at 3:36 PM, Johan Compagner
> <[EMAIL PROTECTED]>
> > > > wrote:
> > > > > yes and add a big javadoc warning that this method is not meant to
> be
> > > > called
> > > > > only meant to be overriden..
> > > > >
> > > > > johan
> > > > >
> > > > >
> > > > > On Wed, Mar 19, 2008 at 11:35 PM, Igor Vaynberg <
> > > > [EMAIL PROTECTED]>
> > > > >
> > > > >
> > > > > wrote:
> > > > >
> > > > > > ok, so given that we make checkrequired protected (pushes it
> into
> > > > 1.5
> > > > > > timeframe) are you ok with moving isrequired() check out into
> > > > > > validaterequired() ?
> > > > > >
> > > > > > -igor
> > > > > >
> > > > > >
> > > > > > On Wed, Mar 19, 2008 at 3:22 PM, Johan Compagner <
> > > > [EMAIL PROTECTED]>
> > > > > > wrote:
> > > > > > > ì guess checkRequired( ) is only overridable because of
> > > > > > FormComponentPanel
> > > > > > > so that again that can be overriden to have there own
> > > > requirement
> > > > > > check?
> > > > > > >
> > > > > > > so yes it should really be at least protected
> > > > > > >
> > > > > > > On Wed, Mar 19, 2008 at 11:10 PM, Igor Vaynberg <
> > > > > > [EMAIL PROTECTED]>
> > > > > > >
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > thats kinda cludge imho. checkrequired() defines the
> process
> > > > of
> > > > > > > > checking, whether that needs to be invoked or not is up to
> the
> > > > > > > > formcomponent and its required attribute.
> > > > > > > >
> > > > > > > > we can make validaterequired() public, although i dont see
> > > > where you
> > > > > > > > would call only that instead of the entire validate()
> > > > pipeline.
> > > > > > > >
> > > > > > > > also right now checkrequired() is only ever called from
> > > > > > > > validaterequired() and i think checkrequired() should not
> be
> > > > public
> > > > > > > > anyways
> > > > > > > >
> > > > > > > > -igor
> > > > > > > >
> > > > > > > >
> > > > > > > > On Wed, Mar 19, 2008 at 3:06 PM, Johan Compagner <
> > > > > > [EMAIL PROTECTED]>
> > > > > > > > wrote:
> > > > > > > > > if you also want to check it in validateRequired() thats
> > > > fine by me
> > > > > > by
> > > > > > > > the
> > > > > > > > > way
> > > > > > > > > But i dont want it to be removed in checkRequired()
> > > > > > > > >
> > > > > > > > > And the javadoc must be updated anyway
> > > > > > > > >
> > > > > > > > > johan
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Wed, Mar 19, 2008 at 11:05 PM, Johan Compagner <
> > > > > > [EMAIL PROTECTED]
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > nope
> > > > > > > > > > i am against that
> > > > > > > > > > validateRequired is protected
> > > > > > > > > > checkRequired is public
> > > > > > > > > >
> > > > > > > > > > And if i want to test for requirement from outside i
> dont
> > > > want
> > > > > > to
> > > > > > > > call
> > > > > > > > > > first for every thing isRequired first
> > > > > > > > > > i find it very odd that a method can return very funny
> > > > stuff
> > > > > > when you
> > > > > > > > dont
> > > > > > > > > > call one method before it first
> > > > > > > > > > it should be self contained
> > > > > > > > > >
> > > > > > > > > > And this way it works for quite some time now, so i
> dont
> > > > really
> > > > > > > > change
> > > > > > > > > > anything.
> > > > > > > > > > it is just stupid to have a method which behavior is
> not
> > > > really
> > > > > > > > defined
> > > > > > > > > > based on that you can call it or not.
> > > > > > > > > > it doesn't make any sense to me to not call
> isRequired()
> > > > in
> > > > > > > > > > checkRequired()
> > > > > > > > > >
> > > > > > > > > > johan
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Wed, Mar 19, 2008 at 10:57 PM, Igor Vaynberg <
> > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > why dont we build that check into validateRequired()
> > > > > > > > > > >
> > > > > > > > > > > so validateRequired() { if (isrequired() {
> ...current
> > > > code } }
> > > > > > > > > > >
> > > > > > > > > > > that way you never have to call checkrequired()
> > > > directly, just
> > > > > > call
> > > > > > > > > > > validaterequired()
> > > > > > > > > > >
> > > > > > > > > > > -igor
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Mar 19, 2008 at 2:54 PM, Johan Compagner <
> > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > i dont agree
> > > > > > > > > > > > then you have to do everywhere
> > > > > > > > > > > >
> > > > > > > > > > > > if (isRequired()) checkRequired()
> > > > > > > > > > > >
> > > > > > > > > > > > thats horrible, checkRequired() can test that
> just
> > > > as fine
> > > > > > > > > > > > i will update the javadoc
> > > > > > > > > > > >
> > > > > > > > > > > > johan
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Mar 19, 2008 at 10:51 PM, Vitaly Tsaplin
> <
> > > > > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > But the javadoc says:
> > > > > > > > > > > > >
> > > > > > > > > > > > > public boolean checkRequired()
> > > > > > > > > > > > > "Checks if the form component's 'required'
> > > > requirement is
> > > > > > met.
> > > > > > > > This
> > > > > > > > > > > > > method should typically only be called when
> > > > > > > > > > > FormComponent.isRequired()
> > > > > > > > > > > > > returns true."
> > > > > > > > > > > > >
> > > > > > > > > > > > > And I agree with javadoc :)
> > > > > > > > > > > > > checkRequired () should be called only to know
> "if
> > > > the
> > > > > > form
> > > > > > > > > > > > > component's 'required' requirement is met". In
> > > > case
> > > > > > > > isRequired()
> > > > > > > > > > > > > returns false this call does not make any
> sense...
> > > > > > > > > > > > >
> > > > > > > > > > > > > Basically if isRequired () returns true you
> know
> > > > that a
> > > > > > > > component
> > > > > > > > > > > > > is required but what you don't know is whether
> the
> > > > > > requirement
> > > > > > > > > > > > > condition is met or not and so to check it out
> you
> > > > call
> > > > > > > > > > > checkRequired
> > > > > > > > > > > > > (). checkRequired () shouldn't call isRequired
> ()
> > > > itself.
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Mar 19, 2008 at 10:39 PM, Johan
> Compagner
> > > > <
> > > > > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > it checks if the required needs to be checked
> > > > and if
> > > > > > that is
> > > > > > > > the
> > > > > > > > > > > case it
> > > > > > > > > > > > > > checks if the input is set
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, Mar 19, 2008 at 10:32 PM, Vitaly
> > > > Tsaplin <
> > > > > > > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > checkRequired () itself shouldn't be
> called
> > > > at all
> > > > > > > > unless
> > > > > > > > > > > > > > > setRequired is true...
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Wed, Mar 19, 2008 at 9:43 PM, Johan
> > > > Compagner <
> > > > > > > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > and did you look at checkRequired?
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > public boolean checkRequired()
> > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > if (isRequired())
> > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Wed, Mar 19, 2008 at 2:24 PM, Vitaly
> > > > Tsaplin <
> > > > > > > > > > > > > > > [EMAIL PROTECTED]>
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hi guys,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > According to the wicket javadoc the
> > > > method
> > > > > > > > > > > checkRequired () of
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > FormComponent class "...should
> typically
> > > > only
> > > > > > be
> > > > > > > > called
> > > > > > > > > > > when
> > > > > > > > > > > > > > > > > isRequired() returns true."
> > > > > > > > > > > > > > > > > But it seems to be different...
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > public final void validate()
> > > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > > validateRequired();
> > > > > > > > > > > <<<-------------------- here
> > > > > > > > > > > > > > > > > if (isValid())
> > > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > >
> convertInput();
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > if (isValid()
> &&
> > > > > > isRequired()
> > > > > > > > &&
> > > > > > > > > > > > > > > > > getConvertedInput() == null &&
> > > > > > > > > > > > > > > > > isInputNullable())
> > > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > >
> > > > > > > > reportRequiredError();
> > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > if (isValid())
> > > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > >
> > > > > > validateValidators();
> > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > protected final void
> > > > validateRequired()
> > > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > > if (!checkRequired())
> > > > > > > > > > > <<<---------------------
> > > > > > > > > > > > > and
> > > > > > > > > > > > > > > here
> > > > > > > > > > > > > > > > > {
> > > > > > > > > > > > > > > > >
> > > > reportRequiredError();
> > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > As you can see the checkRequired ()
> is
> > > > called
> > > > > > > > > > > unconditionally.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Vitaly
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > >
> > > > ---------------------------------------------------------------------
> > > > > > > > > > > > > > > > > To unsubscribe, e-mail:
> > > > > > > > > > > [EMAIL PROTECTED]
> > > > > > > > > > > > > > > > > For additional commands, e-mail:
> > > > > > > > > > > [EMAIL PROTECTED]
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > >
> > > > ---------------------------------------------------------------------
> > > > > > > > > > > > > > > To unsubscribe, e-mail:
> > > > > > > > [EMAIL PROTECTED]
> > > > > > > > > > > > > > > For additional commands, e-mail:
> > > > > > > > [EMAIL PROTECTED]
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > >
> > > > ---------------------------------------------------------------------
> > > > > > > > > > > > > To unsubscribe, e-mail:
> > > > > > [EMAIL PROTECTED]
> > > > > > > > > > > > > For additional commands, e-mail:
> > > > > > [EMAIL PROTECTED]
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > >
> > > > ---------------------------------------------------------------------
> > > > > > > > > > > To unsubscribe, e-mail:
> > > > [EMAIL PROTECTED]
> > > > > > > > > > > For additional commands, e-mail:
> > > > [EMAIL PROTECTED]
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > ---------------------------------------------------------------------
> > > > > > > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > > > > > > For additional commands, e-mail:
> [EMAIL PROTECTED]
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > > > > For additional commands, e-mail: [EMAIL PROTECTED]
> > > > > >
> > > > > >
> > > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > > For additional commands, e-mail: [EMAIL PROTECTED]
> > > >
> > > >
> > >
> >
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
