You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Niall Pemberton <ni...@blueyonder.co.uk> on 2004/04/06 03:20:58 UTC

[Validator] Url Validation Patch - [Bug 28190]

Any feedback on the patch I submitted adding the ability to return an error code from when validating Url's?

  http://issues.apache.org/bugzilla/show_bug.cgi?id=28190


Niall

Re: [Validator] Url Validation Patch - [Bug 28190]

Posted by Niall Pemberton <ni...@blueyonder.co.uk>.
Robert Leland wrote

> Niall Pemberton wrote:
>
> >Any feedback on the patch I submitted adding the ability to return an
error code from when validating Url's?
> >
> >  http://issues.apache.org/bugzilla/show_bug.cgi?id=28190
> >
> >
> >Niall
> >
> >
> I have a deadline this Friday that will probably stretch over the weekend.
> After that I'll have a chance to review it. Looking at it briefly, the
> patch itself looks ok.

OK Thanks.

> The main items I would look at is how it fits in with the other
> validations and if there was
> any mechanism that might be generalized to keep the validations
consistent.

I think all validators should return a "ResultStatus" (currently a protected
inner class in ValidatorResult) which could be made a public class and
expanded to include an error code like this one. That would then provide a
mechanism for this type of scenario and make it easier to add other features
if other scenarios (not that I can think of any) arose in the future.

Niall

> -Rob
>



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Validator] Url Validation Patch - [Bug 28190]

Posted by Niall Pemberton <ni...@blueyonder.co.uk>.
Robert Leland wrote:

> Robert Leland wrote:
>
> > Niall Pemberton wrote:
> >
> >> Any feedback on the patch I submitted adding the ability to return an
> >> error code from when validating Url's?
> >>
> >>  http://issues.apache.org/bugzilla/show_bug.cgi?id=28190
> >>
> >>
> >> Niall
> >>
> >>
> >>
> > I have a deadline this Friday that will probably stretch over the
> > weekend.
> > After that I'll have a chance to review it. Looking at it briefly, the
> > patch itself looks ok.
> > The main items I would look at is how it fits in with the other
> > validations and if there was
> > any mechanism that might be generalized to keep the validations
> > consistent.
> >
> > -Rob
>
> I am inclined to simplify the code and always return the expandedCode,
> that would make the control flow simpler.

It would but not by much, the difference would be mean replacing eight
statements like

       return expandedCode ? errorCode : ERROR_SCHEME;
with
       return errorCode;

and then it would mean more downstream logic to convert the "expanded code"
to a more general one with a set of range checks. Also a small additional
benefit was that it enabled me to keep the same method name "isValid" as the
existing method by including a additional boolean parameter.

...but I'm easy going. If when you've had time to look you still want it
changed, let me know and I'll submit another patch.

Also once you're happy with the patch, I'll also update the tests and submit
a patch for that.

Niall



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Validator] Url Validation Patch - [Bug 28190]

Posted by Robert Leland <rl...@apache.org>.
Robert Leland wrote:

> Niall Pemberton wrote:
>
>> Any feedback on the patch I submitted adding the ability to return an 
>> error code from when validating Url's?
>>
>>  http://issues.apache.org/bugzilla/show_bug.cgi?id=28190
>>
>>
>> Niall
>>
>>  
>>
> I have a deadline this Friday that will probably stretch over the 
> weekend.
> After that I'll have a chance to review it. Looking at it briefly, the 
> patch itself looks ok.
> The main items I would look at is how it fits in with the other 
> validations and if there was
> any mechanism that might be generalized to keep the validations 
> consistent.
>
> -Rob 

I am inclined to simplify the code and always return the expandedCode, 
that would make the control flow simpler.


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Validator] Url Validation Patch - [Bug 28190]

Posted by Robert Leland <rl...@apache.org>.
Niall Pemberton wrote:

>Any feedback on the patch I submitted adding the ability to return an error code from when validating Url's?
>
>  http://issues.apache.org/bugzilla/show_bug.cgi?id=28190
>
>
>Niall
>
>  
>
I have a deadline this Friday that will probably stretch over the weekend.
After that I'll have a chance to review it. Looking at it briefly, the 
patch itself looks ok.
The main items I would look at is how it fits in with the other 
validations and if there was
any mechanism that might be generalized to keep the validations consistent.

-Rob


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org