Commons Validator
  1. Commons Validator
  2. VALIDATOR-203

Refactor UrlValidator - especially the line 370-ish TODO.

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4.0 Release
    • Component/s: Routines
    • Labels:
      None

      Description

      The UrlValidator class needs some cleanup. Just looking at VALIDATOR-202 there were some obvious places where things could be cleaned up and the solution for VALIDATOR-202 could use improvement.

        Activity

        Hide
        Niall Pemberton added a comment -

        Ben has added a new InetAddressValidator - so UrlValidator should use that if/when its refactored - see VALIDATOR-241

        Show
        Niall Pemberton added a comment - Ben has added a new InetAddressValidator - so UrlValidator should use that if/when its refactored - see VALIDATOR-241
        Hide
        Niall Pemberton added a comment -

        I deleted the new version of this validator in the routines" package and then re-added it again but copying so that the commit history is preserved:

        http://svn.apache.org/viewvc?view=rev&revision=588102
        http://svn.apache.org/viewvc?view=rev&revision=588103

        Show
        Niall Pemberton added a comment - I deleted the new version of this validator in the routines" package and then re-added it again but copying so that the commit history is preserved: http://svn.apache.org/viewvc?view=rev&revision=588102 http://svn.apache.org/viewvc?view=rev&revision=588103
        Hide
        Ben Speakmon added a comment -

        I've taken out the domain name and IP validating and delegated that to the new InetAddress/DomainValidator. It's definitely better; once I get a chance to start replacing the oro regexes, some other possible simplifications will probably pop up.

        Show
        Ben Speakmon added a comment - I've taken out the domain name and IP validating and delegated that to the new InetAddress/DomainValidator. It's definitely better; once I get a chance to start replacing the oro regexes, some other possible simplifications will probably pop up.
        Hide
        Niall Pemberton added a comment -

        Thanks for doing this Ben, same comment as per Email valdiator

        IMO it should improve performance if the regular expressions are not compiled for each URL validated. The JavaDoc for java.util.regex.Pattern[1] indicates that patterns are thread safe - so would be better to just compile the regular expression once:

        "All of the state involved in performing a match resides in the matcher, so many matchers can share the same pattern."
        [1] http://java.sun.com/j2se/1.4.2/docs/api/java/util/regex/Pattern.html

        Show
        Niall Pemberton added a comment - Thanks for doing this Ben, same comment as per Email valdiator IMO it should improve performance if the regular expressions are not compiled for each URL validated. The JavaDoc for java.util.regex.Pattern [1] indicates that patterns are thread safe - so would be better to just compile the regular expression once: "All of the state involved in performing a match resides in the matcher, so many matchers can share the same pattern." [1] http://java.sun.com/j2se/1.4.2/docs/api/java/util/regex/Pattern.html
        Hide
        Ben Speakmon added a comment -

        Yep – like I said in VALIDATOR-242, no problem turning the Patterns into static fields. I'll get that when I have a minute.

        Show
        Ben Speakmon added a comment - Yep – like I said in VALIDATOR-242 , no problem turning the Patterns into static fields. I'll get that when I have a minute.
        Hide
        Ben Speakmon added a comment -

        Patterns changed. We okay with resolving this?

        Show
        Ben Speakmon added a comment - Patterns changed. We okay with resolving this?
        Hide
        Henri Yandell added a comment -

        Yup, looks good.

        Show
        Henri Yandell added a comment - Yup, looks good.

          People

          • Assignee:
            Ben Speakmon
            Reporter:
            Henri Yandell
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development