• The Coward (unregistered)

    Besides either using IsDigit or a whitelist of chars, we allow anything in the text, simply because people can type

    +44 (0)1234 56789

    or

    01234 56789 ext 3

    etc.

    Depends what you really need it for.

    When I get asked for my phone number (usually for marketing spam) I put 0100 100000 which is good enough to defeat most form checks.

  • James (unregistered)

    Then somebody comes along and adds the following for for a small software test

    ©®

  • James (unregistered) in reply to The Coward

    No you don't do that. You put in a voip number that you get inbound calls deducted from your outbound call's :)

    That way the marketing spam pays for your outbound calls :)

  • (cs)

    Did this programmer not go to school or something?

  • lesle (unregistered)

    Ma Bell's Officially Recommended Exchange Names.

    I use CApital 4-6195. Other than occasionally being asked how to make a capital 4, what's the problem?

    http://ourwebhome.com/TENP/Recommended.html

    Ma Bell's Officially Recommended Exchange Names

  • Neil (unregistered)

    Of course the right way to validate the phone number is to hook a modem up to the server and dial it.

  • chris (unregistered)

    Simple. Put a Java class called VirtualPhone behind the page, with a typeKey(char keyValue, String contextBeanId) method which throws an exception if the key value is invalid, a simulateCall() method that throws if the number is invalid, and a getLastNumberDialled() method. Don't ask me about how exceptions are handled because I neither know nor care how the UI works.

  • Anon (unregistered) in reply to Your Name
    Your Name:
    You may want to do other validation for the phone number as well. For the US, for instance, you'll want to make sure it's ten digits. Also, some checking of the actual digits:

    abb-cdd-xxxx

    Neither A nor C can be equal to 1, and neither BB nor DD can be equal to 11.

    You know, just for a start. I mean, if you're not doing anything useful with the field you don't need to bother, I guess.

    IIRC A and C also cannot be 0.

  • Design Pattern (unregistered)

    Scala:

    filter(_ isDigit) mkString
    
  • SG_01 (unregistered) in reply to Anon
    Anon:
    Your Name:
    You may want to do other validation for the phone number as well. For the US, for instance, you'll want to make sure it's ten digits. Also, some checking of the actual digits:

    abb-cdd-xxxx

    Neither A nor C can be equal to 1, and neither BB nor DD can be equal to 11.

    You know, just for a start. I mean, if you're not doing anything useful with the field you don't need to bother, I guess.

    IIRC A and C also cannot be 0.

    Unless the phone number starts with 110, 00 or +, in which case the next 1 - 4 numbers are a country code, and the rest is country dependant ^^

  • flyboyfred (unregistered) in reply to Groogy
    Groogy:
    Don't think it is Javascript, looks like ruby looking at the comment.

    I think every CodeSOD should specify the language.

  • keith (unregistered)

    What, no backwards compatibility? "Pennsylvania Six Five Thousand!"

    http://en.wikipedia.org/wiki/PEnnsylvania_6-5000

  • (cs) in reply to flyboyfred
    flyboyfred:
    Groogy:
    Don't think it is Javascript, looks like ruby looking at the comment.

    I think every CodeSOD should specify the language.

    For extra lols when they specify the wrong one?

  • SG_01 (unregistered) in reply to NUXI
    NUXI:
    The real WTF is that this code is wrong because letters are allowed in phone numbers...

    replace("a", "") should actually be replace("a", "2")

    Except you'd get in trouble with p and w, as those characters actually have a meaning for dialers. I believe the functional set of characters is something along the lines of:

    [0-9*#pw+]

    Though for readability the following set is added:

    [ -(),]

    (I might have missed some, but those are the more common ones)

  • SG_01 (unregistered) in reply to SG_01
    SG_01:
    NUXI:
    The real WTF is that this code is wrong because letters are allowed in phone numbers...

    replace("a", "") should actually be replace("a", "2")

    Except you'd get in trouble with p and w, as those characters actually have a meaning for dialers. I believe the functional set of characters is something along the lines of:

    [0-9*#pw+]

    Though for readability the following set is added:

    [ -(),]

    (I might have missed some, but those are the more common ones)

    Erm, that last one should be [ -(),] of course ^^

  • Mark (unregistered)

    Dude. Google "regular expressions". I swear it will blow your effing mind.

  • (cs)

    LOL! It should obviously have been

    homephone = homephone.replace(" ", "").replace("-", "").replace("(", "").replace(")", "")
    .replace("[", "").replace("]", "").replace(".", "")
    .replace("`", "").replace("^", "").replace("!", "")
    .replace("@", "").replace("#", "").replace("$", "")
    .replace("%", "").replace("&", "").replace("*", "")
    .replace("+", "").replace("=", "").replace("/", "")
    .replace("<", "").replace(">", "").replace(":", "")
    .replace(";", "").replace("~", "").replace("A", "")
    .replace("B", "").replace("C", "").replace("D", "")
    .replace("E", "").replace("F", "").replace("G", "")
    .replace("H", "").replace("I", "").replace("J", "")
    .replace("K", "").replace("L", "").replace("M", "")
    .replace("N", "").replace("O", "").replace("P", "")
    .replace("Q", "").replace("R", "").replace("S", "")
    .replace("T", "").replace("U", "").replace("V", "")
    .replace("W", "").replace("X", "").replace("Y", "")
    .replace("Z", "").replace("a", "").replace("b", "")
    .replace("c", "").replace("d", "").replace("e", "")
    .replace("f", "").replace("g", "").replace("h", "")
    .replace("i", "").replace("j", "").replace("k", "")
    .replace("l", "").replace("m", "").replace("n", "")
    .replace("o", "").replace("p", "").replace("q", "")
    .replace("r", "").replace("s", "").replace("t", "")
    .replace("u", "").replace("v", "").replace("w", "")
    .replace("x", "").replace("y", "").replace("z", "") 
    

    All those extra assignments are so unnecessary!

  • (cs) in reply to BentFranklin
    BentFranklin:
    programmer = programmer.replace("right away")

    Or maybe:

    employees.replace("programmer", "monkey");

  • airdrik (unregistered) in reply to toth

    He should have used functions for all of that redundant code:

    def fixhomephone(homephone):
        return homephone.replace(" ", "").replace("-", "").replace("(", "").replace(")", "")
    .replace("[", "").replace("]", "").replace(".", "")
    .replace("`", "").replace("^", "").replace("!", "")
    .replace("@", "").replace("#", "").replace("$", "")
    .replace("%", "").replace("&", "").replace("*", "")
    .replace("+", "").replace("=", "").replace("/", "")
    .replace("<", "").replace(">", "").replace(":", "")
    .replace(";", "").replace("~", "").replace("A", "")
    .replace("B", "").replace("C", "").replace("D", "")
    .replace("E", "").replace("F", "").replace("G", "")
    .replace("H", "").replace("I", "").replace("J", "")
    .replace("K", "").replace("L", "").replace("M", "")
    .replace("N", "").replace("O", "").replace("P", "")
    .replace("Q", "").replace("R", "").replace("S", "")
    .replace("T", "").replace("U", "").replace("V", "")
    .replace("W", "").replace("X", "").replace("Y", "")
    .replace("Z", "").replace("a", "").replace("b", "")
    .replace("c", "").replace("d", "").replace("e", "")
    .replace("f", "").replace("g", "").replace("h", "")
    .replace("i", "").replace("j", "").replace("k", "")
    .replace("l", "").replace("m", "").replace("n", "")
    .replace("o", "").replace("p", "").replace("q", "")
    .replace("r", "").replace("s", "").replace("t", "")
    .replace("u", "").replace("v", "").replace("w", "")
    .replace("x", "").replace("y", "").replace("z", "") 
    
    def fixworkphone(workphone):
        ...
    
    def fixcellphone(cellphone):
        ...
    
    homephone = fixhomephone(homephone)
    workphone = fixhomephone(workphone) # issue 64286
    cellphone = fixcellphone(cellphone)
    
    
  • Jay (unregistered)

    So, say,

    "7,{2'
    

    is a valid phone number?

  • (cs) in reply to @Deprecated
    @Deprecated:
    BentFranklin:
    programmer = programmer.replace("right away")
    Or maybe:

    employees.replace("programmer", "monkey");

    I'm guessing that's how they ended up with this code in the first place.

  • (cs)
    1. Idea
      number = number.replaceAll("\D",""); no to short and who knows \D anyway?

    2. Idea number = number.replaceAll("[^\d"],""); yes but what if reader ist not that fluent in regexp, got to remove these predefined groups

    3. Idea number = number.replaceAll("[^0123456789]"],""); OK but what if the reader does not know about negation...

    4. idea see article

    perfect!!!

  • Fred (unregistered) in reply to BLah Blah
    BLah Blah:
    I get it! It's more flexible! Just in case they decide to allow or disallow another character, they simple add or remove another .replace()! brillant!
    But that would require recompiling! Better to put the forbidden chars in a database, named, perhaps, "blacklist".

    Programming to a whitelist is too hard, plus, it's not secure!

  • Anonymous (unregistered) in reply to Mark
    Mark:
    Dude. Google "regular expressions". I swear it will blow your effing mind.
    I'm worried about the amount of people who are suggesting regular expressions for such a trivial problem. Regular expressions have their place but they are totally the wrong tool for a simple non-numeric replace like this. Fun fact: in the .NET framework the regular expression handling classes have the highest cyclomatic complexity of any classes in the entire framework (last time I checked, circa 3.0). Yes, I know this code isn't .NET but it highlights an inescapable fact about regexes - they are a seriously expensive operation, in any language. Don't abuse them for silly little jobs like this one.
  • caper (unregistered)

    The WTF is the requirement "Only the numbers should be saved."

    Why would you do that if you don't bother to validate ? You may as well just save the input exactly as given.

  • (cs) in reply to Shoruke
    Shoruke:
    Did this programmer not go to school or something?
    You're asking about preschool?
  • Jerry (unregistered) in reply to caper
    caper:
    The WTF is the requirement "Only the numbers should be saved."
    Oh is that what he was trying to do?

    Easy. Add zero. If it isn't a number you'll get an error.

  • (cs) in reply to Anonymous
    Anonymous:
    Mark:
    Dude. Google "regular expressions". I swear it will blow your effing mind.
    I'm worried about the amount of people who are suggesting regular expressions for such a trivial problem. Regular expressions have their place but they are totally the wrong tool for a simple non-numeric replace like this. Fun fact: in the .NET framework the regular expression handling classes have the highest cyclomatic complexity of any classes in the entire framework (last time I checked, circa 3.0). Yes, I know this code isn't .NET but it highlights an inescapable fact about regexes - they are a seriously expensive operation, in any language. Don't abuse them for silly little jobs like this one.

    True. But the sense of smugness you get from using regexes is priceless.

  • (cs) in reply to Jerry
    Jerry:
    caper:
    The WTF is the requirement "Only the numbers should be saved."
    Oh is that what he was trying to do?

    Easy. Add zero. If it isn't a number you'll get an error.

    Wow. That's isn't the same thing at all.

  • (cs) in reply to frits
    frits:
    Even VB can do it better:
        Private Function GetPhoneNumber(ByVal homephone As String) As String
            Dim temp As String = String.Empty
            Dim tempVal As Integer
            Dim i As Integer
            For i = 0 To homephone.Length - 1
                tempVal = Val(homephone(i))
                If ((Not tempVal = 0) And (Not (homephone(i) = "0"(0)))) Then
                    temp += CUInt(tempVal).ToString
                End If
            Next
            GetPhoneNumber = tempVal
        End Function
    
    You can be one demented fuck when you want to be.
  • ben (unregistered) in reply to Groogy

    "Don't think it is Javascript, looks like ruby looking at the comment."

    In ruby you use "gsub" for this purpose; "replace" does something a bit different (and fairly pointless).

  • Vacaloca (unregistered) in reply to frits
    frits:
    Jerry:
    caper:
    The WTF is the requirement "Only the numbers should be saved."
    Oh is that what he was trying to do?

    Easy. Add zero. If it isn't a number you'll get an error.

    Wow. That's isn't the same thing at all.

    Obvious troll is obvious

  • C-Octothorpe (unregistered) in reply to hoodaticus
    hoodaticus:
    frits:
    Even VB can do it better:
        Private Function GetPhoneNumber(ByVal homephone As String) As String
            Dim temp As String = String.Empty
            Dim tempVal As Integer
            Dim i As Integer
            For i = 0 To homephone.Length - 1
                tempVal = Val(homephone(i))
                If ((Not tempVal = 0) And (Not (homephone(i) = "0"(0)))) Then
                    temp += CUInt(tempVal).ToString
                End If
            Next
            GetPhoneNumber = tempVal
        End Function
    
    You can be one demented fuck when you want to be.

    Mwahahahahahaha... Beware, looking at the below code may induce vomiting.

    
    foreach (char number in phoneNumber)
                {
                    try
                    {
                        int temp = Convert.ToInt32(number);
                        result = result + (string)temp.ToString() as string;
                    }
                    catch
                    { 
                        // This should never happen...
                    }
                }
    

    oh, and can't forget the foreach-switch...

                foreach (char number in phoneNumber)
                {
                    switch (number)
                    { 
                        case '1':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '2':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '3':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '4':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '5':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '6':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '7':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '8':
                            result = result + (string)number.ToString() as string;
                            break;
                        case '9':
                            result = result + (string)number.ToString() as string;
                            break;
                        default:
                            break;
                    }
                }
    
    
  • blarg (unregistered) in reply to Bryan the K

    F@#)%(*@# you askimet.

    why have a shitty capture and then prevent so many legitimate posts?

    Implement a proper captcha and get rid of askimet.

  • blarg (unregistered) in reply to NUXI
    NUXI:
    The real WTF is that this code is wrong because letters are allowed in phone numbers...

    replace("a", "") should actually be replace("a", "2")

    Hint - it's called a telephone NUMBER for a reason. You can draw whatever characters you want on your handset and press those until your finger tips wear away, but the number is still a number.

  • Anonymous (unregistered) in reply to frits

    Note also that the code calls replace so many time that regex performance issues pale in comparison.

  • (cs) in reply to hoodaticus
    hoodaticus:
    frits:
    Even VB can do it better:
        Private Function GetPhoneNumber(ByVal homephone As String) As String
            Dim temp As String = String.Empty
            Dim tempVal As Integer
            Dim i As Integer
            For i = 0 To homephone.Length - 1
                tempVal = Val(homephone(i))
                If ((Not tempVal = 0) And (Not (homephone(i) = "0"(0)))) Then
                    temp += CUInt(tempVal).ToString
                End If
            Next
            GetPhoneNumber = tempVal
        End Function
    

    .

    You can be one demented fuck when you want to be.
    Thanks. I feel I really embraced the VB way of doing things.
  • blarg (unregistered) in reply to Anonymous
    Anonymous:
    Mark:
    Dude. Google "regular expressions". I swear it will blow your effing mind.
    I'm worried about the amount of people who are suggesting regular expressions for such a trivial problem. Regular expressions have their place but they are totally the wrong tool for a simple non-numeric replace like this. Fun fact: in the .NET framework the regular expression handling classes have the highest cyclomatic complexity of any classes in the entire framework (last time I checked, circa 3.0). Yes, I know this code isn't .NET but it highlights an inescapable fact about regexes - they are a seriously expensive operation, in any language. Don't abuse them for silly little jobs like this one.

    Are you one of those guys who spends 3 days finding a 'clever' workaround to save 2 clock cycles for a task which is called once an hour and has a disk/network bottleneck anyway?

    This is precisely the type of task for which a regex should be used. Seriously Expensive is a relative term, and given the context of this system - no it isn't.

  • Autocracy (unregistered)

    A Møøse once bit my sister ... No realli!

  • Vacaloca (unregistered) in reply to C-Octothorpe
    C-Octothorpe:
    Mwahahahahahaha... Beware, looking at the below code may induce vomiting.

    Obviously you should have put each character into a string array and used String.Join to make the final string.

    Maybe even writing your own linked list class and dropping in some unsafe code with pointers in there.

    This is not spam. Akismet can kiss my ass.

  • PsyckoBaton (unregistered)

    Where is death when we need it !

  • (cs) in reply to PsyckoBaton
    PsyckoBaton:
    Where is death when we need it !

    It's here, in the form of Akismet.

  • C-Octothorpe (unregistered) in reply to Vacaloca
    Vacaloca:
    C-Octothorpe:
    Mwahahahahahaha... Beware, looking at the below code may induce vomiting.

    Obviously you should have put each character into a string array and used String.Join to make the final string.

    Maybe even writing your own linked list class and dropping in some unsafe code with pointers in there.

    This is not spam. Akismet can kiss my ass.

    No, that would be too 'enterprisey'... You need to show an absolute lack of understanding with both gerenal concepts and of the language specific usage/functions.

    You want to show someone lifting a car with their bare hands to change a tire, not them building an elevator out of bubble-gum and sticks.

  • MadX (unregistered) in reply to Rob

    Paid by the character...

  • Mike D (unregistered)

    Apparently he didn't know how to use functions, either.

    I love seeing code in triplicate. I love seeing code in triplicate. I love seeing code in triplicate.

  • (cs) in reply to Anonymous
    Anonymous:
    Regular expressions have their place but they are totally the wrong tool for a simple non-numeric replace like this. Blah blah blah...it highlights an inescapable fact about regexes - they are a seriously expensive operation, in any language.
    You say that like it's a bad thing. Is execution time the only value by which you measure one tool's "total wrongness" vs. another?
  • davee123 (unregistered) in reply to Mike D

    I love how he even managed to miss the OBVIOUS standard ASCII characters:

    ',_{}|"?

    He couldn't even manage to write bad code :(

    DaveE

  • Macro King (unregistered) in reply to frits
    frits:
    Even VB can do it better:
            For i = 0 To homephone.Length - 1
    

    You forgot OnError, ResumeNext. And you're going to need that because VB strings are 1 based!

    The real WTF etc etc...

  • David Skoll (unregistered) in reply to Anonymous

    What do you mean? That code looks like it was auto-generated...

  • (cs) in reply to David Skoll

    I'd love to see a full 16-bit Unicode compliant version of this code. And one compatible with escaped unicode and escaped HTML...and escaped HTML Unicode.

Leave a comment on “No Letters Allowed!”

Log In or post as a guest

Replying to comment #:

« Return to Article