• fluffy (unregistered)

    I'd like to see what test cases this guy threw at this code. Not to mention that the != stripping wouldn't work (it'll just remove the !).

    Also, personally I'd have compactified it a bit, by just going through the original string a char at a time and adding it to the return value instead (if it was allowed to be in anyway), assuming no regex/sed-type function was available. (I don't know what Java provides along those lines.)

  • G Dawg (unregistered)

    Granted this is a long piece of code, but it runs faster and more efficiently than the equivalent (i.e. using methods), and given the correct input it will not go into an infinite loop at all.

  • Max (unregistered)

    @fluffy:

    The stripping of th != is no problem at all. Because the script is searching for an ! a few lines before - so this code wouldn't be executed that soon. And even if it just strips the ! of the != the = is removed just some lines (and infinities) later.

    Don't know whats your problem.

  • Hexy (unregistered)

    I'm gonna go ahead and disagree with you on this one G Dawg. Did you read the editor's note? Because the Remove() function only returns the modified string, and does not modify it directly, then the occurrence of ANY search character will cause an infinite loop. iPos will never equal -1 because Criteria never changes. The only time this code will work is if there are no search characters at all. Efficiency is no substitute for code that actually works.

  • Hassan Voyeau (unregistered)

    This is not Java, most likely C#.

  • AvonWyss (unregistered)

    Yeah, that's C#. A one-liner to solve that "problem":
    return System.Text.RegularExpressions.Regex.Replace(Criteria, @"[*<>!&|?=]", string.Empty);

    And I bet that this would be way faster than the (fixed) version of the code above, due to all the string allocations and copying...

  • Jake Vinson (unregistered)

    "and given the correct input it will not go into an infinite loop at all"

    So you'd have a text box, a submit button, and a note that says "please don't include the following characters: $, %, ... as it will crash our software. OK, thanks!" I love putting validation in the user's hands.

  • Phil Scott (unregistered)

    Maybe they had a regular expression validator control, and then it called this function "just to be sure." And idealy, the regular expression validator took care of preventing bad input, so this function never entered a case during their testing that made everything go ape shit crazy. I'm not apologizing for the code, but I'm willing to bet that's how things went down.

    Other than that, I'm about 99% G Dawg is being sarcastic or just screwing with people because yesterday he (or she) was also quick to approve of that bastard of a round function.

  • skicow (unregistered)

    No disrespect towards Ade, but how long do you have to be a programmer at his company to be deemed a 'senior' programmer?!? 6 weeks?

    I guess since this code was written by a 'senior' programmer it didn't need to be tested AT ALL!

  • Hassan Voyeau (unregistered)

    I am curious to know why the search characters are being stripped anyway?

  • Heath Stewart (unregistered)

    Ah, the pains of relying on IntelliSense over documentation!

  • WanFactory (unregistered)

    At least the method wasn't named StripSearchAllCharacters

  • Jeff S (unregistered)

    >>At least the method wasn't named StripSearchAllCharacters

    :)

  • RJ (unregistered)

    I've encountered several people (including myself) who didn't immediately realize that .net strings are immutable. It is a shame that the definition of the class didn't mark the methods as constant. In C++ the signature of the function could be:

    String* String::Remove(int, int) const;

    But, I'm not sure if the C#/VB support const methods.

  • rs (unregistered)

    As no one has provided the specs, we cannot assume that the result of this function was not the desired result. Perhaps the developer's only flaw is his/her poor judgment in naming functions.

    I therefore would like to submit the following function. Same results, less filling.

    private static string StripAllSearchCharacters(string Criteria)
    {
    while (0 < 1);
    }

    This should work except for extremely large values on 0 or extremely small values of 1.

  • Tim Cartwright (unregistered)

    rs, he was probably getting paid on LOC... :)

  • Rick (unregistered)

    "Sir, we have a problem. An evil user could hack our DB by entering search characters".
    "Can't you fix that?"
    "No, but I could minimize the effects and turn the DB-hack into a simple DOS attack."
    "Very well. do it."

  • Lore Weaver (unregistered)

    Hehehe, the users would at least have plenty of time to reflect on their use of certain characters.

  • josh (unregistered)

    This guy's ahead of his time! He's using quantum programming methods: in universes where invalid data is passed to this function, it does not return. Thus you are ensured that you will only have valid data after calling it. Optimally you'd destroy the universe on failure, but computers haven't quite advanced to that level yet.

  • Uwe Keim (unregistered)

    I thought it is called "Department", not "Departent"? :-)

  • Nikolay Simeonov (unregistered)

    Very nice. And looks so damn familiar to me... I keep talking to my guys when they have the same code twice they should not do cut-n-paste but put it in a function like this:

    private static string _StripAllSearchCharacters(string Criteria, string SearchFor)
    {
    iPos = Criteria.IndexOf(SearchFor);

    while (iPos != -1)
    {
    Criteria = Criteria.Remove(iPos, 1);
    iPos = Criteria.IndexOf(SearchFor);

    }
    return Criteria;
    }


    private static string StripAllSearchCharacters(string Criteria)
    {
    Criteria = _StripAllSearchCharacters(Criteria, "");
    Criteria = _StripAllSearchCharacters(Criteria, "<");
    Criteria = _StripAllSearchCharacters(Criteria, ">");
    ........
    return Criteria;
    }


    Ok, I agree, the one-liner using regular expressions is much better, but some people never heard of that and I wouldn't mind seeing the code above and be even happier to see perhaps something slightly better like this:

    private static string StripAllSearchCharacters(string Criteria)
    {
    string SearchFor = "    <>!&|?=";

    for (int ndx=0; ndx<SearchFor.Length; ndx++)
    Criteria = _StripAllSearchCharacters(Criteria, SearchFor[ndx]);
    return Criteria;
    }

  • Vitani (unregistered)

    Re: rs

    surely

    private static string StripAllSearchCharacters(string Criteria)
    {
    while (true);
    }

    would surfice?

  • Clinton Pierce (unregistered)

    After a long time of working with Perl and C, .Net's "immutable strings" methods have bitten me quite a few times.

    Granted, I usually catch them on the first run-through with the debugger.

    What would be nice in C# is a warning when a function with no side effects like this (such as a String Func(String)) is run and the return value is ignored. Of course the "no side effects" is left as an exercise for the implementer. :)

  • rs (unregistered)

    Re: Vitani

    That would definitely work (or, should I say, give the same result), but "true" is 4 keystrokes and "0<1" is 3 keystrokes (I know, in my example I added spaces, but ...). I don't know about you but I would certainly prefer to type 81 keystrokes (including returns) instead of 82. Remember, when creating stupid code, alway think of the confort of the developer! Those saved keystrokes will pay-off in more stupid code and future entertainment in this forum. :)

  • Ludvig (unregistered)

    re: rs

    Saved keystrokes? Okay, here you go:
    #define StripAllSearchCharacters(v) while(1);

    I know #define's. Trust me.

  • Nemanja Trifunovic (unregistered)

    While it is the programmer's fault because he didn't read the documentation, I still think the naming in the .NET String class is very bad. If you call a non-static method "Remove", it implies that something is removed from the instance of the called object.

  • Grazelda Stankovich (unregistered)

    I tried this on my local machine and it does wonders. I'm planning on suggesting this code at my next implementation meeting. We're probably going to introduce this logic into all of our production code.

  • Captain Obvious (unregistered)

    Ok geniuses, if you're so smart, then how is it you don't have logic to better handle your user comment postings? If you include a URL with your name it turns your name blue and appears as a link. But if you only submit your name (with no URL) then it still appears as a link (albeit black) when you mouse over it. LAME. And you have the nerve to critique others...

  • Jeff (unregistered)

    @Captain Obvious -- you need to upgrade from Netscape 2.0, I think.

  • Captain Obvious (unregistered)

    Actually, I'm using the latest version of Firefox. Take that!

  • Jerry Pisk (unregistered)

    Even a shorter one (and still a real method):

    private static string StripAllSearchCharacters(string Criteria)
    {
    for(;;);
    }

  • Grazelda Stankovich (unregistered)

    After a quick meeting with the implementation group, this code was deployed on 3 Linux servers and crashed all of them. One machine refuses to boot. I would not recommend using this code for production purposes.

  • George Prado (unregistered)

    I would use a good internet browser instead if I had that lame of an issue... if you are captain obvious you would notice that... or is it captain oblivious?.. just my $0.02...

    programmers like this that give us bad names... I doubt there is any other job on earth where you can do hit or miss work like this and still make it... /sob

    vive le C#!
    french is the language of love...
    for everything else there's C#

  • Captain Obvious (unregistered)

    Even the Captain has his off days.

  • foxyshadis (unregistered)

    Actually, it's not a 'link' just an style that underlines on hover. The .Text dudes put it in for misguided consistency, I guess. Any modern browser will show it that way; take it up with the .Text guys. You can't expect someone who has an otherwise default .Text install to customize away one little thing that annoys just you.

    The comments here are hilarious, but most of the infinite loops miss the possibility that the string contains none of the characters and finishes! You need a
    if(rand(100)>60) while(1); else return Criteria;

  • peter beaguely (unregistered)

    i like lasso. wish it had this kind of functionality.

  • matt (unregistered)

    @ George Prado:

    Meteorologists come to mind. :D

  • gn0m (unregistered)

    > #define StripAllSearchCharacters(v) while(1);

    Can raise at least an "extraneous ';'" warning (CodeWarrior) and some sort of inf. loop warning (MSVC8, use for(;;) ). So I'm afraid it never even gets a chance to turn into an infinite loop at my setup (warnings are errors) ;).

  • Sean McVey (unregistered)

    > #define StripAllSearchCharacters(v) while(1);

    More importantly, this kind of #define trickery is not allowed in C# only in C/C++...

    The access modifier has no colon following it (not C++, could be C#/Java)
    Unless there's some serious jiggery-pokery going on, chances are that the programmer hasn't defined and used a new string class, so "string" implies C# (Java's is "String")

  • Sean McVey (unregistered)

    Damn... thought mailto: would work :(
    Now I look dumb

  • (unregistered) in reply to Sean McVey

    <FONT style="BACKGROUND-COLOR: #efefef">Come on guys... think like an OO programmer. Maybe he extended string and overwrote Remove to actually remove the characters... Maybe his new string type is called SpecialStringThatRemoves : String</FONT>

    Of course, his method "StripAllSearchCharacters" would only work when a string of the type <FONT style="BACKGROUND-COLOR: #efefef">SpecialStringThatRemoves was passed in... but that is how you stay Senior Programmer. (Or am I missing something here?)</FONT>

    <FONT style="BACKGROUND-COLOR: #efefef">Oh yeah... forgot that string is a sealed class (declared final for the java guys)... you can always hope though. :)</FONT>

  • Anonymous (unregistered) in reply to Clinton Pierce

    What would be nice in C# is a warning when a function with no side effects like this (such as a String Func(String)) is run and the return value is ignored. Of course the "no side effects" is left as an exercise for the implementer. :)

    The Erlang programming language forces you to do something with every return value.  It has a special variable "_", which is used explicitly to lie to the compiler that you're doing something with a return value.  All values assinged to _ are ignored.  This at least forces you to be clued in that you are dropping some value on the floor.

  • Robin Message (unregistered) in reply to Clinton Pierce
    Anonymous:

    What would be nice in C# is a warning when a function with no side effects like this (such as a String Func(String)) is run and the return value is ignored. Of course the "no side effects" is left as an exercise for the implementer. :)


    Try Ruby.

    It has a standard (admittedly not enforced by compiler/interpreter) that mutator methods should end in "!" and query methods in "?". So lots of string methods (for example) have equivalents with a ! after them to change strings in place.

Leave a comment on “From the "It worked when I tested it" Department”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article