- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Oh how silly: Everybody knows this should be coded as:
Always good to unroll loops. It speeds it up.
Admin
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?
Admin
@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.
Admin
There's a glaring omission! The original 0s are not replaced! This is a security nightmare!
Admin
do it recursively, until there is nothing left to replace.
Admin
Wait... if they're on the ground floor, why not just let him jump? Seems easier and not very dangerous.
Admin
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."
Admin
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.
Admin
at lest he did not use regual expressions
Admin
I would have thought: "Visual [expletive] BASIC" would be the preferred nomenclature. But, to each, his or her own.
Admin
I think that shuffles feet might wrings hands be how I'd do it bats eyes. What's the better way?
Admin
Callback hell in VB...
Admin
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
Admin
@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';
Admin
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 FunctionAdmin
TRWTF is the fact that the comment box allows you to enter more characters than is actually allowed to be submitted for a comment
Admin
Ah, yes those do seem better. Thanks.
Admin
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 :)
Admin
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 ;-)
Admin
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.
Admin
Eventually, the data is saved as 0's and 1's somewhere, just replace all those 1's with 0 and you are there ;)
Admin
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.
Admin
That's also how you have to do it in SQL, isn't it?
Admin
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.
Admin
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.
Admin
Frankly,
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.
Admin
It works, assignment to the function name returns the last assigned value when the function returns. VB, it is wonderful!
Admin
"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.
Admin
@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.
Admin
That's clever AF.
Admin
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.
Admin
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.
Admin
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.
Admin
Yup, "not a disaster" is the usual PL/SQL standard for code quality.
Admin
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.
Admin
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.
Admin
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.
Admin
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.
Admin
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.
Admin
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.
Admin
What do you do with the letters contained in the string then? The routine converts "ABC123" to "ABC000".
Admin
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.)
Admin
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
Admin
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
Admin
ReplaceNumbersWithZeros = "00000000000";
done.
Admin
You haven't seen the requirements, either, though.
x="000"could meet them just fine.Admin
oh hell no.. in SQL you'd do something like
return translate(strText,'123456789','000000000')
Admin
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.
Admin
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....
Admin
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 :
i'm damn sure the coder must have another thought of WTF he wrote; at the time of writing... :D