"Creative" coding



  • I knew I was in trouble when the other employees started apologizing for the code on my first day.  It seems that the original development team - almost none of whom were still with the company - had taken the approach "if there are two ways to do something, do it in the most convoluted way possible."  Oh, and "if there's only one way to do something, invent a second way, then follow rule #1."  This attitude extended all the way from the system architecture to individual code statements.

    Although daily - or hourly - WTF moments were the norm, I concluded that the following sample just about says it all.  This has been anonymized somewhat, but the underlying issues - as well as the helpful comment - are intact.

    private const string NAME_CHARS = @"[a|b|c|d|e|f|g|h|i|j|k|l|m|n|o|p|q|r|s|t|u|v|w|x|y|z|A|B|C|D|E|F|G|H|I|J|K|L|M|N|O|P|Q|R|S|T|U|V|W|X|Y|Z|0|1|2|3|4|5|6|7|8|9|'|\-| ]+";
    

    private bool NameIsValid(string strTest)
    {
    // TODO: The following if block should be reduced to one return statememt:
    // return Regex.IsMatch(strTest, NAME_CHARS);
    if (!Regex.IsMatch(strTest, NAME_CHARS))
    return false;
    else
    return true;
    }



  •  I cringed, fell on my desk and whined. What's wrong with [a-zA-Z] or even [abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ] !? All the | does is add '|' to the searchable character list... 52 times! And I thought my regexps were bad!

     If you ever find whoever coded this, be sure to hit them over the head with "Regular expressions for dummies". 



  • @Ren said:

    What's wrong with [a-zA-Z] or even [abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ] !?

    Those wouldn't include:

    "0123456789'-| "

    Though it would be easy enough to fix that.



  • @Thief^ said:

    @Ren said:

    What's wrong with [a-zA-Z] or even [abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ] !?

    Those wouldn't include:

    "0123456789'-| "

    Though it would be easy enough to fix that.

     

      Ohh! My window cut the text at  T|U| so I assumed it just went on to Y|Z]. I guess I fail. Yeah, [A-Za-z0-9\-' ]+, also that fails with scandinavian letters such as äö, and even more with unicode ;)



  • Am I the only one bothered by the TODO:?

    @GalacticCowboy said:

    // TODO: The following if block should be reduced to one return statememt:
    // return Regex.IsMatch(strTest, NAME_CHARS);

    It's not as if the todo was "See if this can be reduced" but rather, it spells out exactly what needs to be replaced. Rather than replacing it, though, he just left a note that something should be done.



  • @Ren said:

    Ohh! My window cut the text at  T|U| so I assumed it just went on to Y|Z]. I guess I fail. Yeah, [A-Za-z0-9\-' ]+, also that fails with scandinavian letters such as äö, and even more with unicode ;)

     

    You fail with escaping.  Backslash doesn't work inside a bracket expression.  From man regex: [i]"To include a literal '-', make it the first or last character, or the second endpoint of a range."[/i]



  •  Can't we do a simple isalnum or something? Whatever language this is, it's sure to have an equivalent function, and without the overhead of a regex.  Plus it should work in whatever locale your using.



  • @tdb said:

    @Ren said:

    Ohh! My window cut the text at  T|U| so I assumed it just went on to Y|Z]. I guess I fail. Yeah, [A-Za-z0-9\-' ]+, also that fails with scandinavian letters such as äö, and even more with unicode ;)

     

    You fail with escaping.  Backslash doesn't work inside a bracket expression.  From man regex: "To include a literal '-', make it the first or last character, or the second endpoint of a range."

    ~$ perl -e '$_ = "-"; print "x\n" if(/[a\-b]/);'

    You fail at man pages. ;)



  • I've found similar misuse of regular expressions in a web site I'm currently maintaining, which was originally outsourced on a shoestring budget. In several situations where a single reg exp could have done the job, the original developer instead used repetitive PHP code around very basic reg exps which individually did nothing that a substring search couldn't do.



    It seems that there's a class of bottomfeeder programmers who claim to be able to use tools like reg exps, yet in reality lack the intelligence to use them to anything near their full potential.



  • There's so many WTF's in this one.
    I would write it as:

    private static readonly Regex NameChars = new Regex(@"[a-zA-Z0-9'\- ]+", RegexOptions.Compiled);
    
    private static bool NameIsValid(string strTest)
    {
        return NameChars.IsMatch(strTest);
    }
    

    In summary:

    • string constant, but why not make the Regex object constant instead?
    • method should be static
    • the stupid comment, and the way it uses return


  • @Ren said:

    From man regex: "To include a literal '-', make it the first or last
    character, or the second endpoint of a range."

    What do POSIX regex functions have to do with .NET ones?

    @Ren said:

    ~$ perl -e '$_ = "-"; print "x\n" if(/[a-b]/);'

    Ditto for Perl.

    For the record, MSDN Library says that character groups in .NET regexes can include escaped chars.



  • @tdb said:

    You fail with escaping.  Backslash doesn't work inside a bracket expression.  From man regex: "To include a literal '-', make it the first or last character, or the second endpoint of a range."

     

    I think most people are referring to PCRE's when they talk about regexes. (Regices?)



  • @SlyEcho said:

    There's so many WTF's in this one.
    I would write it as:

    private static readonly Regex NameChars = new Regex(@"[a-zA-Z0-9'\- ]+", RegexOptions.Compiled);
    
    private static bool NameIsValid(string strTest)
    {
        return NameChars.IsMatch(strTest);
    }

    Does IsMatch do anchored matching, or does that regex match any string that contains at least one alphanumeric?



  • @Carnildo said:

    Does IsMatch do anchored matching, or does that regex match any string that contains at least one alphanumeric?

    Anything with 1 alphanumeric :P. But so does the original post so he has reproduced the same functionality. I suspect the original author thought he'd written the equivalent of @"^[a-zA-Z0-9'\- ]+$"

    Edit: Left off the @ symbol.



  • @fyjham said:

    @Carnildo said:

    Does IsMatch do anchored matching, or does that regex match any string that contains at least one alphanumeric?

    Anything with 1 alphanumeric :P. But so does the original post so he has reproduced the same functionality. I suspect the original author thought he'd written the equivalent of @"^[a-zA-Z0-9'- ]+$"

    Edit: Left off the @ symbol.


    The double negation in the original may be traces of someone trying to correct the match, but giving up before arriving at !/[^A-Z0-9'- ]/i (perl version -- but the code would have to check for empty strings as well).



  • @savar said:

    I think most people are referring to PCRE's when they talk about regexes. (Regices?)
    Regexes. Regex is an abbreviation of REGular EXpressions. It is not a proper Latin word, hence it does not need to be declented.



  • @DrJokepu said:

    @savar said:
    I think most people are referring to PCRE's when they talk about regexes. (Regices?)
    Regexes. Regex is an abbreviation of REGular EXpressions. It is not a proper Latin word, hence it does not need to be declented.

    The psuedo-intellectual use of Latin declension for non-Latin words is very common with computer nerds, like one of those mind virii.

     

    Where is Welpog?  He's missing out on a chance to talk about Latin! 


  • BINNED

    @morbiuswilters said:

    The psuedo-intellectual use of Latin declension for non-Latin words is very common with computer nerds, like one of those mind virii.

     

    Agreed, but the same probably goes for pseudo-.



  • @topspin said:

    @morbiuswilters said:

    The psuedo-intellectual use of Latin declension for non-Latin words is very common with computer nerds, like one of those mind virii.

     

    Agreed, but the same probably goes for pseudo-.

    You miss joke. 



  •  No, he replied directly to him...



  • @ActionMan said:

    No, he replied directly to him...

    I take it you aren't smart enough to get the joke either.  It's not that hard, seriously. 



  • @morbiuswilters said:

    The psuedo-intellectual use of Latin declension for non-Latin words is very common with computer nerds, like one of those mind virii.

     

    None of those are half as irritating as "boxen".  I'm glad I've never had somebody say that to me in person, because if I had, I'd probably be in a cell now.

    What's funny is that I've never seen it done for index.



  • @Aaron said:

    @morbiuswilters said:

    The psuedo-intellectual use of Latin declension for non-Latin words is very common with computer nerds, like one of those mind virii.

     

    None of those are half as irritating as "boxen".  I'm glad I've never had somebody say that to me in person, because if I had, I'd probably be in a cell now.

    What's funny is that I've never seen it done for index.

     

    What, you mean "indices"?



  • @RocketRick said:

    @Aaron said:

    @morbiuswilters said:

    The psuedo-intellectual use of Latin declension for non-Latin words is very common with computer nerds, like one of those mind virii.

     

    None of those are half as irritating as "boxen".  I'm glad I've never had somebody say that to me in person, because if I had, I'd probably be in a cell now.

    What's funny is that I've never seen it done for index.

     

    What, you mean "indices"?

    No, no, indexenices.



  • @morbiuswilters said:

    No, no, indexenices.
    indiix?


Log in to reply