• Little Bobby Tables (unregistered)

    Oh how silly: Everybody knows this should be coded as:

    Replace (strText, "1000", "0000");
    Replace (strText, "1001", "0000");
    Replace (strText, "1002", "0000");
    :
    :
    Replace (strText, "9999", "0000");
    Replace (strText, "100", "000");
    Replace (strText, "101", "000");
    Replace (strText, "102", "000");
    :
    :
    Replace (strText, "999", "000");
    Replace (strText, "10", "00");
    Replace (strText, "11", "00");
    Replace (strText, "99", "00");
    Replace (strText, "1", "0");
    Replace (strText, "2", "0");
    :
    :
    Replace (strText, "9", "0");
    

    Always good to unroll loops. It speeds it up.

  • Valhar2000 (unregistered)

    The only way I can think of to do this in VB that wouldn't be this ugly is to use Regex. Is there another way?

  • Yog-Sothoth (unregistered)

    @Bobby Xkcd: I hope you are aware that your code doesn't work. You aren't doing anything with the strings returned by Replace(). Worth noting that MSDN's Replace() example amounts to the same thing as Remy's WTF: Replace(("1","0").Replace("2","0").Replace("3","0"), etc.

  • j-random-ngr (unregistered)

    There's a glaring omission! The original 0s are not replaced! This is a security nightmare!

  • henk (unregistered)

    do it recursively, until there is nothing left to replace.

  • Robert Morson (google)

    Wait... if they're on the ground floor, why not just let him jump? Seems easier and not very dangerous.

  • ray10k (unregistered)

    With that introduction, I fully expected something like, "split the string into individual characters, then run each character through an if-else-if pyramid to check if it's a number, replace by zero if so."

  • Little Bobby Tables (unregistered) in reply to Yog-Sothoth

    Yeah well that's up to the test team to decide. I've written the code, I've wiped my backside and flushed, my work is over. Anything else is in the hands of maintenance.

  • not a robot (unregistered)

    at lest he did not use regual expressions

  • Peter (unregistered)

    I would have thought: "Visual [expletive] BASIC" would be the preferred nomenclature. But, to each, his or her own.

  • Blushing Bride (unregistered)

    I think that shuffles feet might wrings hands be how I'd do it bats eyes. What's the better way?

  • kitihounel (unregistered)

    Callback hell in VB...

  • Tim (unregistered)

    I don't see the WTF here - as a functional solution without using regex, it looks pretty good to me, assuming performance isn't a priority

  • Brian (unregistered)

    @Valhar, @Bride The most straightforward solution to me, without ugly regexes and inefficient loops (pseudocode here, because I wouldn't touch VB with an 11.5-foot pole):

    for (char& c in str) if (isNum(c)) c = '0';

  • (nodebb) in reply to Blushing Bride

    As this code is using Hungarian notation, I expect this is not VB.Net, but an older dialect.

    Rem VB6/VBA example
    Private Function ReplaceNumbersWithZeros(ByVal strText As String) As String
        Dim Position as Integer
    
        For Position = 1 To Len(strText)
            Select Case Mid$(strText, Position, 1)
            Case "1" To "9"
                Mid$(strText, Position, 1) = "0"    ' Replace the character in-place.
            End Select
        Next Position
        ReplaceNumbersWithZeros = strText
    End Function
    
  • TRWTF (unregistered)

    TRWTF is the fact that the comment box allows you to enter more characters than is actually allowed to be submitted for a comment

  • Blushing Bride (unregistered)

    Ah, yes those do seem better. Thanks.

  • Dave (unregistered) in reply to Brian

    Snort. More coffee needed? Just get the length of the input string and write a string of zeros that long. If you want to mess about VB style, start with one zero and use padleft :)

  • Pedro (unregistered)

    It's not so bad, it works. But this simple example illustrates how Regex is not so bad as people like to paint it: strText = Regex.Replace(strText, "\d", "0");

    ... it even replaces the zeroes with different zeroes, taking care of that glaring security issue ;-)

  • WTFGuy (unregistered)

    Makes you wonder if there's unused domain knowledge that all input strings consist purely of digits. Then this would be a real WTF versus the much simpler [generate a string of zeros of the appropriate length] and return that. My VB is ancient rusty, but something like Return String(Length(strInput), "0")

    Which also solves the pesky security hole mentioned above. ;)

    It seems they've ignored the I18n implications. There are lots of Unicode characters that represent digits that are not the traditional ASCII 0x30 - 0x39. All those will slip through the sieve of Replace calls.

  • Ruud (unregistered)

    Eventually, the data is saved as 0's and 1's somewhere, just replace all those 1's with 0 and you are there ;)

  • Pjrz (unregistered) in reply to Pedro

    Unfortunately your solution may replace the original zeros, but it also replaces all the digits with the SAME zero. This clearly leaves your solution open to a reverse-engineering hack. Shame on you.

  • Barry Margolin (google)

    That's also how you have to do it in SQL, isn't it?

  • hoser (unregistered) in reply to Barry Margolin

    Oracle PL/SQL developer here - yes, that's how I'd do it. Regexes could do it, but the performance impact of 9 nested REPLACEs are negligible enough that this isn't a disaster. Looping character by character is unneeded complexity. We have code similar to this in a few places in our code base.

  • Simon Clarkstone (unregistered)

    There was a Underhanded C Code Contest entry based on this text transformation (with a more sensible implementation) a few years ago. The contest was to write a program to censor an image by blacking out some rectangles at given coordinates, but you had to engineer a backdoor into the program that somehow leaked the information is was supposed to be censoring, and the code must stand up to casual code review. The input format was a simple one where colours were represented as comma-separated decimal integers.

    One entry was a clearly-written program with no obvious obscurity or fancy tricks; it kept track of where it was in the image, and if it was supposed to be inside a censored rectangle it replaced all digits with "0". This meant, in the common case of there being no leading zeros in the numbers in the input, that bright colour channels were censored to "000", dull ones to "00", and very dark ones to "0". E.g. when censoring black text on a white background, you can undo the censoring by changing the "0"s all back to "2"s and get clearly readable text back out.

  • wow (unregistered)

    Frankly,

    1. Does it pass unit tests?
    2. Are there performance issues?

    I don't see the WTF. it took someone about 1 minute to code. If that person doesn't know regex, perhaps they should have spent an hour doing this "the right way" but if there were 100 features in the pipe needing development I don't see the problem.

  • Remco (unregistered) in reply to Yog-Sothoth

    It works, assignment to the function name returns the last assigned value when the function returns. VB, it is wonderful!

  • (nodebb) in reply to Nutster

    "As this code is using Hungarian notation, I expect this is not VB.Net, but an older dialect." What else do you think it is? C#6? My colleagues will stay with "Hungarian Notation" forever.

  • Yog-Sothoth (unregistered)

    @Remco OK, but Bobby doesn't make an assignment to the function name - so it doesn't work. Replace returns a new string which is a copy of the input string with the replacements performed. In Bobby's code, that new string is never assigned to anything. For his code to work, each line would need to look like this: strText = Replace(strText, "<blah>", "<new_blah"); Then, although (intentionally) ridiculous, it would work.

  • siciac (unregistered) in reply to Simon Clarkstone

    That's clever AF.

  • TrollingMagician (unregistered) in reply to Nutster

    Didn't you hear? Microsoft just announced that everything else is depreciated and Hungarian notation is recommended for variable naming? Oh wait, I mean Silverlight is no longer supported. No, I mean ADO is the thing to use, not DAO.

  • Selma (unregistered)

    VB does not have Regex or a simple equivalent. Therefore this code is actually pretty reasonable. What alternatives are there? Character by character examination and building a new string? Whatever you choose it won't be particularly efficient.

    Given that you don't know the context (how big the strings were, how fast it actually needed to be) the criticisms seem a little churlish to me.

    At the very least this is semi-encapsulated in a function which means you can test it, benchmark it and then change it if it does indeed prove too slow or memory hungry.

    VB wasn't a bad system- it just had it's limitations.

  • TrollingMagician (unregistered) in reply to Dave

    6 bottles of milk! Dave, based on the first three sentences that Remy gave as a spec, there's nothing to indicate that the string doesn't also contain non-numeric characters.

  • siciac (unregistered) in reply to hoser

    Oracle PL/SQL developer here - yes, that's how I'd do it. Regexes could do it, but the performance impact of 9 nested REPLACEs are negligible enough this isn't a disaster.

    Yup, "not a disaster" is the usual PL/SQL standard for code quality.

  • Jeremy Hannon (google)

    Been there, done that. In Visual Basic versions PRIOR to .Net (VB5, VB6, VBA, etc.), I don't think there was a better, more efficient way to do what they are tying to do. Yes, it would create a lot of copying and duplicating in memory, but those versions were never known for their speed and efficiency anyway.

    The only thing that would be "more efficient" would be assigning the results of each one to the variable before passing it again. But, if it was VB6, etc. you would need to remember to deallocate any additional variables you created to make sure the memory might actually get cleaned up. Not sure if it was me or the product but there was a lot of "magic" to get things to work in those days than the predictable nature of .Net.

  • Regurgitated rubbish (unregistered) in reply to wow

    If that person doesn't know regex,

    Regex isn't native to VB6/VBA so you'd need to do an API call.

    Cascading Replace() like that would be a bit slow due to all the string handling, code like Nutster posted would be faster.

    This is really an example of "Perfect is the enemy of good" rather than a WTF; simple to understand, no dependencies, and it works.

  • Kashim (unregistered)

    To all those who say this isn't a WTF:

    It is. Just because there might be certain circumstances where you could see yourself doing this doesn't mean you didn't cringe when you saw it. That cringe, that reflexive shudder, that fundamental feeling that "this is wrong" is what makes this a WTF. It's that even the thought of calling Replace() 9 times, though perfectly straightforward and with no dependencies, reeks in the minds of computer scientists who abhor doing the same thing twice if it isn't in a function.

    It is that fundamental "This should be better." that makes this a WTF. Heck, before doing this, I might break down the string into tokens myself, replace en-masse with an OR, and then reassemble, just to know that at least I was only passing over the string ONCE.

  • Mr A (unregistered) in reply to Kashim

    Your sensibilities are noble but in the face of commercial reality would you really say

    "This is ugly and we need to change the entire system because it forces clumsy coding"

    What we have here is a simple to understand function. The alternative is a more complex solution that is undoubtedly more efficient but probably doesn't need to be. Repeated enough times, these sensibilities drive software costs through the roof.

    Sometimes we just need the simple solutions.

  • ZZartin (unregistered)

    There's nothing wrong with this, nested replace calls are pretty common especially in languages that either A) don't have support for regex or B) it's some kind of formula where you can't loop. It's also fairly common when you need to do multiple replaces that wouldn't really be acceptable in a regex or looping mechanism, so yeah this is fine.

  • Dave (unregistered) in reply to Jeremy Hannon

    Of course it's a wtf. Not because there's a better way to do it, but because there's no need to do more than count the characters in the string and write a new string with that many zeroes.

  • Regurgitated rubbish (unregistered) in reply to Dave

    write a new string with that many zeroes.

    What do you do with the letters contained in the string then? The routine converts "ABC123" to "ABC000".

  • Regurgitated rubbish (unregistered) in reply to Kashim

    I might break down the string into tokens myself

    Seriously? How long it that going to take and how many bugs it it going to have when it 's finished? We've people like Dave above who can't even figure out what it's supposed to be doing. Dave thinks "ABC123" should be returned as "000000".

    As before, why do 'perfect' when 'good enough' works just fine? (I'd do a regex, but anyway.)

  • guest (unregistered)

    Private Function ReplaceNumberWithZero(ByVal strText As String) As String For i = 1 To Len(strText) If IsNumeric(Mid$(strText, i, 1)) Then Mid$(strText, i, 1) = "0" Next ReplaceNumberWithZero = strText End Function

  • guest (unregistered)

    Private Function ReplaceNumberWithZero(ByVal strText As String) As String

    For i = 1 To Len(strText) If IsNumeric(Mid$(strText, i, 1)) Then Mid$(strText, i, 1) = "0" Next ReplaceNumberWithZero = strText

    End Function

  • (nodebb)

    ReplaceNumbersWithZeros = "00000000000";

    done.

  • siciac (unregistered) in reply to Regurgitated rubbish

    We've people like Dave above who can't even figure out what it's supposed to be doing.

    You haven't seen the requirements, either, though. x="000" could meet them just fine.

  • Anti-Hoser (unregistered) in reply to hoser

    oh hell no.. in SQL you'd do something like

    return translate(strText,'123456789','000000000')

  • pk (unregistered)

    This article is a prime example of what is wrong with the people here... "Duh, just write a bunch of zeros..." - That's not what the function does "Duh, just use a regex..." - Not available in the context "Duh, just think and write a 10 line function for this..." - You are not getting paid to waste such time. "But you go over the string 9 times!" - I envy you that you have the luxury to concern yourself with this.

  • The true WTF? (unregistered)

    Tail recursion, so the function calls are no more work than any repeated function call. Avoids copying the target object to a new object, so less work than repeated function calls.

    So it's ugly, but uses the language quickly and efficiently.

    And if you're so shallow that you need to be talked down from a ledge after seeing some clear, efficient, and ugly code, then they shouldn't be talking you down from the ledge at all....

  • death the kid (unregistered)

    a simple regex is good in place of nasty lengthy code of taking care of each numerals.

    i'd go with this code: str = str.replaceAll("[1-9]", "0"); or if ignoring regex i'd use :

       for(int i=1; i<=9; i++)
            str = str.replaceAll(Integer.toString(i), "0");
    

    i'm damn sure the coder must have another thought of WTF he wrote; at the time of writing... :D

Leave a comment on “Got Your Number”

Log In or post as a guest

Replying to comment #:

« Return to Article