Oh, sorry. I meant "write in javadoc" :)
On Thu, Mar 20, 2008 at 2:02 AM, Vitaly Tsaplin <[EMAIL PROTECTED]> wrote: > For me that's probably fine if you will clearly right in javadoc > that anyone who overrides checkRequired should carefully call > isRequired in order to conform the common behavior. > > > > On Thu, Mar 20, 2008 at 1:33 AM, Johan Compagner <[EMAIL PROTECTED]> wrote: > > 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] > > > > >