The Daily WTF: Curious Perversions in Information Technology

2007-01-22 Reply Admin

Sean:
Foo:
Strings in Java are immutable, so every time s = s.substring(1) is called it is creating a brand new string and copying the substring into that new string. If this utility code is running on the server and it's in a high volume application that's a lot of extra load on the garbage collector.

Not exactly. String.substring() uses a package level constructor on the String class that allows a substring to share the data of its parent string. It does not need to copy the data each time because it knows that the object it is passing the data array to is another immutable string. Anyway, that doesn't make the code any less of a wtf since Long.parseLong() could have been used on the string with leading zeroes.

but, what you are not getting is the fact that new String objects are created every time substring is called.

To everybody spewing out code that is (in most cases) even worse than the original, look at the String.trim() implementation to see how one trims characters off a String.

newfweiler · 2007-01-22 Reply Admin

Trondster:

In Java, JavaScript and others, a leading zero signifies an octal number, just like the prefix "0x" signifies Hex.. Hint: Always specify 10 as the radix.. ;)

That in itself is a WTF. How many computers use octal? (Bits grouped in threes -- great for 18-bit words, sucks for 16-bit words.) The last computers I touched that used octal were the GE 225 and CDC 8090, and neither one ran JavaScript or Java. Why do new languages carry on the tradition of octal? I can see the phone company maintaining a building-filling Strowger switch for the one old lady who still has a dial phone, but it doesn't build new ones.

2007-01-22 Reply Admin

Our code goes to eleven!

2007-01-22 Reply Admin

We've once had a similar bug with octal numbers in TCL program which caused a program not to work correctly from 8am till 10am. Nobody was testing the program at this period of time, neither did customers run it at evenings, so the bug managed to hide for about two years :)

2007-01-22 Reply Admin

Trondster:
That's why I prefer proper {}'s..

Where "proper" == "I get paid by the line".

Also known as KLOC Indenting.

rmr · 2007-01-22 Reply Admin

waefafw:
Bad programmers makes baby Jesus cry.

Amen to that. I think some people missed the hand warmers WTF from last week.

2007-01-22 Reply Admin

Let's make it better! (Even converted to C# for you)

		long StringToLong(string s)
		{
			for (int j = 0; j < s.Length; j++)
			{
				if (s.ToCharArray()[0] == '0')
				{
					StringBuilder newstr = new StringBuilder(s.Length - 1);
					char[] chars = s.ToCharArray();
					for (int i = 1; i < chars.Length; i++)
						newstr.Append(chars[i]);
					return StringToLong(newstr.ToString());
				}
				else
					return long.Parse(s);
			}
			return StringToLong("000011");
		}

2007-01-22 Reply Admin

Dunno about the StringToLong method, but ChatAt() is a new one to me. Silly me, using CharAt() all this time, never knowing...

2007-01-22 Reply Admin

Wow. Just wow.

Trimming leading 0s the non-WTF way:

String trimLeadingZeros(String string) {
    // Store the string length - 1 so it doesn't need to
    // be calculated every pass through the loop.

    // The length is decremented as we only care about the
    // leading zeros and by definition the last character
    // is not a leading character.

    // This allows "000" to return "0" and causes the empty
    // string to return "" as expected.

    int length = string.length() - 1;

    // We need i after we exit the loop, as it will be the
    // index of the first non-zero character before the last
    // character.
    int i;

    for (i = 0; i < length; i++) {
        if (string.charAt(i) != '0') {
            break;
        }
    }

    // String.substring(0) is in fact smart enough to
    // return itself, so this won't allocate a new object
    // for that case.  Since substring checks anyway,
    // there's no point in checking here.

    return string.substring(i);
}

There. No string objects allocated every single pass through the loop, no need to repeatedly call length() because the string keeps changing, simple.

And, as already mentioned, trimming leading 0s before calling Long.parseLong is worthless. For compactness, here's a repeat without the comments:

String trimLeadingZeros(String string) {
    int length = string.length() - 1;
    int i;
    for (i = 0; i < length; i++) {
        if (string.charAt(i) != '0') {
            break;
        }
    }
    return string.substring(i);
}

(Also, as an additional WTF, the forum software adds "
" to text located within "

" tags. So Firefox-users will see extra lines in the code, thanks to that. I suppose I should open a RFE for Firefox to ignore 
s in  when in Quirks mode. Of course, that wouldn't help here, since the forums are in Strict mode.)

2007-01-22 Reply Admin

public static long parseLong(String s) throws NumberFormatException Parses the string argument as a signed decimal long. The characters in the string must all be decimal digits, except that the first character may be an ASCII minus sign '-' (\u002D') to indicate a negative value. The resulting long value is returned, exactly as if the argument and the radix 10 were given as arguments to the parseLong(java.lang.String, int) method. Note that neither the character L ('\u004C') nor l ('\u006C') is permitted to appear at the end of the string as a type indicator, as would be permitted in Java programming language source code.

Parameters: s - a String containing the long representation to be parsed Returns: the long represented by the argument in decimal. Throws: NumberFormatException - if the string does not contain a parsable long.

BrownHornet · 2007-01-22 Reply Admin

Sick Boy:
I think everyone's forgeting that the best way to solve any problem is a case statement.
int j = 0;

while(s.CharAt(j) == '0') j++;

switch(j) { case 1: s=s.substring(1);break; case 2: s=s.substring(2);break; case 3: s=s.substring(3);break; case 4: s=s.substring(4);braek; etc... }

return Long.parseLong(s);

At my last job, someone actually wrote the following code to do the opposite (zero-pad a string to 10 digits):

switch (s.length())
{
    case 1: s = "000000000" + s; break;
    case 2: s = "00000000" + s; break;
    case 3: s = "0000000" + s; break;
    ...
    case 9: s = "0" + s; break;
}

I knew he was going to need to pad another string to 40 digits later on (which he did not know), so I had to point out the WTFness of his code:

Me: What are you going to do when you when you need to pad to 40 digits? Him: When would you ever need to pad something to 40 digits realistically? Me: The TCN is 40 digits, and it has to be zero padded. Him: Ummm... I guess I could use a loop.

2007-01-22 Reply Admin

  for ( int i = 1 ; i <= 10 ; i++ )
  {
      /* do something important */
      if ( condition ) i = 11;
      /* do something else important */
  }

should be refactored into:

  bool lastLoop = false;
  for ( int i = 1 ; i <= 10 && !lastLoop ; i++ )
  {
      /* do something important */
      bool lastLoop = ( condition );
      /* do something else important */
  }

so that we don't rely upon 'magic' properties of the loop variable. Yes, it takes an extra byte, but it is so much clearer.

2007-01-22 Reply Admin

Looks like a job for a variant on Duff's Device...

switch (s.length()) {
     case 1: s = "0" + s;
     case 2: s = "0" + s;
     case 3: s = "0" + s;
     case 4: s = "0" + s;
     case 5: s = "0" + s;
     case 6: s = "0" + s;
     case 7: s = "0" + s;
     case 8: s = "0" + s;
     case 9: s = "0" + s;
}

I'm so ashamed.

2007-01-22 Reply Admin

I am so so sorry guys. I just have to do this.

long parseLong(String str) { if(str != null && str.length() > 0) { if(str.charAt(0) == '0') return parseLong(str.substring(1)); else return Long.parseLong(str,10); }

return FILE_NOT_FOUND;

}

Mcoder · 2007-01-22 Reply Admin

[quote user="sys<in"] What if the spec was "if condition X occurs, set the value of i to 11, do something-else-important, and exit the loop"? [/quote]

Then you have a problem, since as soon as the loop ends, the value of i will be 12. Maybe something-else-important uses i, but you can simply put the parts that use it inside the if.

[quote user="sys<in"] Another possibility is that the value of I has no bearing in the commented-out parts, but the condition else can only be tested at the point where the if test was inserted. [/qoute]

Then you use a boolean, it may also be usefull if something-else-important uses i.

[quote user="sys<in"] Without knowing what the removed parts parts do, there is no way of knowing if that is a WTF or not. [/quote]

Messing with loop indexes is ALWAYS a WTF. Some compilers will behave badly if you do that, some languages requires nasty side effects by doing that and most people can't deal with the consequences. Even if none of that apply, your code becomes messy.

Why quote doesn't work on preview?

gwenhwyfaer · 2007-01-22 Reply Admin

Bunny:
Curious, what would be a better way to do it?

Your comment makes me very sad.

(...But not as sad as the reply page. I use links-hacked, and I'm typing this reply in a 2x20 textbox.

Ouch.)

2007-01-22 Reply Admin

57451:
I am so so sorry guys. I just have to do this.

Why are you apologising? That's a perfectly natural tail-recursive implementation. It's not your fault that crap programming languages like Java don't support pattern matching.

strToLong ("0":s) = strToLong s strToLong s = parseLong s 10

2007-01-22 Reply Admin

I love people who think they're smarter than they actually are.

Anon:

String trimLeadingZeros(String string) {
    int length = string.length() - 1;
    int i;
    for (i = 0; i < length; i++) {
        if (string.charAt(i) != '0') {
            break;
        }
    }
    return string.substring(i);
}

If you're going to call that the "non-WTF way" you might want to avoid a for loop that contains a single if statement with a break. Anyone who ever creates for loops like that should just give up calling themselves a programmer, they've proven they have absolutely no clue.

public static String trimLeadingZeros(String string) {
    int length = string.length() - 1;
    int i;
    for (i = 0; i < length && string.charAt(i) == '0'; i++) ;
    return string.substring(i);
}

For contains a conditional already. No need to add another if statement for absolutely no reason.

2007-01-22 Reply Admin

Well, the only reason I could see (at first glance) was that there might have been some concern if the string contained more significant digits than could be represented in a long.

However, a 30 second test showed that parseLong seems to handle an arbitrary number of leading zeroes - and if you're interested in looking at the J2SE source you'll see a better (faster and more general) way of dealing with that situation.

chrismcb · 2007-01-22 Reply Admin

verisimilidude:
  for ( int i = 1 ; i <= 10 ; i++ )
  {
      /* do something important */
      if ( condition ) i = 11;
      /* do something else important */
  }
should be refactored into:
  bool lastLoop = false;
  for ( int i = 1 ; i <= 10 && !lastLoop ; i++ )
  {
      /* do something important */
      bool lastLoop = ( condition );
      /* do something else important */
  }
so that we don't rely upon 'magic' properties of the loop variable. Yes, it takes an extra byte, but it is so much clearer.

I hope you don't go and build a new factory every time you seem something strange. (who cam up with the term "refactor" anyways?)

Without know what is in /* do something else important */ this code can't really be rewritten. What if there is code in / * do something else important */ Such has foo = i%2; or if (i<10)....

Out of all the WTF's I've seen, this is fairly minor. Can it be rewritten to be better and cleaner? Perhaps, but at least it works, and works without doing a ton of extra processing.

Samah · 2007-01-22 Reply Admin

Trondster:
That's why I prefer proper {}'s.. Properly {}'ed:
  foreach ( Term t in current ) 
  {
      if ( category == "" )
      {
          result = t;
          break;
      }
      else if ( t.category == category )
      {
         result = t;
      }
  }
  if ( result == null ) 
  {
    return;
  }
  return result;
It's not that bad - without a break it returns the last item found instead of the first.. ;)

..And shouldn't that be 'charAt' and not 'chatAt'? ;)

Might I also point out here that technically the "else" keyword isn't even needed here, as the first clause of the "if" block forcibly exits the execution of the loop. In Java (yes I know that code example isn't) if you have a really nazi compiler (or you've configured yours to be) it will throw a warning here for unnecessary code.

Also, I don't know how C# works in terms of return statements, but in the case of Java the mismatch of "return;" and "return value;" will give you a nice compile error, regardless of what the return type (or lack of) is for the method.

VGR · 2007-01-22 Reply Admin

Rich:
I wonder how many WTFs could be avoided by a quick look at the API... =)

Thousands.

Prefect · 2007-01-22 Reply Admin

Does the java parseLong function ignore leading whitespace? If so, the 0 stripping is going to give the wrong value for something like "0 1".

2007-01-22 Reply Admin

newfweiler:
Trondster:

In Java, JavaScript and others, a leading zero signifies an octal number, just like the prefix "0x" signifies Hex.. Hint: Always specify 10 as the radix.. ;)

That in itself is a WTF. How many computers use octal? (Bits grouped in threes -- great for 18-bit words, sucks for 16-bit words.) The last computers I touched that used octal were the GE 225 and CDC 8090, and neither one ran JavaScript or Java. Why do new languages carry on the tradition of octal? I can see the phone company maintaining a building-filling Strowger switch for the one old lady who still has a dial phone, but it doesn't build new ones.

It would be a WTF if it was true, but it isn't (for java, at least).

The original poster is confusing strings with (e.g. s = "00123") with numeric literals (e.g. s = 00123). The numeric literal is interpreted as octal, but if you parse the string it will be interpreted as decimal.

gwenhwyfaer · 2007-01-22 Reply Admin

That in itself is a WTF. How many computers use octal? (Bits grouped in threes -- great for 18-bit words, sucks for 16-bit words.)

Well, if you use octal to encode the bytes of the x86 instruction set, it makes it a bit more logical...

Just out of interest, I wonder how many PDP-10-clone cores could be squeezed into the silicon occupied by a modern x86?

2007-01-22 Reply Admin

Richie:
long StringToLong( String s ) { for ( int j = 0 ; (j < s.length()) && ( s.chatAt(0) == '0' ); j++ ) { s = s.substring(1);
} return Long.parseLong( s ); }

Maybe it works, but doesn't seem to be the correct way to do that. A better way:

while(s.length() > 0 && s.charAt(0) == '0') s = s.substring(1);

2007-01-23 Reply Admin

LizardKing:
What is wrong with the StringToLong method?

The fact that Long.parseLong handles leading zeros:
public class PL {
    public static void main(String[] args) {
        String s = "00004567";
        System.out.println("String '" + s + "' as a long " + Long.parseLong(s));
    }
}
No loop necessary, despite the idiotic posts that use charAt and substring (they're WTF material themselves).

Too true. But lets just say that they've got a wonderfully non-compliant java runtime. (lets give the curtesy of the doubt here). They should have known that duplicating a string is expensive.

long StringToLong( String s ) { int i = 1; for (; i =< s.length(); ++i) { if ( s.charAt(i) != '0' ) { break; } } s = s.substring(i);

return Long.parseLong( s ); }

besides has anyone caught the typo s.chatAt(0) ?

CAPTCHA: alarm (I certainly am if this is the quality of code driving places like my bank!)

2007-01-23 Reply Admin

Samah:

Also, I don't know how C# works in terms of return statements, but in the case of Java the mismatch of "return;" and "return value;" will give you a nice compile error, regardless of what the return type (or lack of) is for the method.

Finally somebody noticed. No, C# doesn't allow that either.

Heh. Took me longer to get through the replies to see if nobody's picked up on that than to check the spec.

-Lars

Earl Purple · 2007-01-23 Reply Admin

Martin:
What is wrong with the StringToLong method?
The loop just remove all the zeros from the start of the string. (Yes it is a stupid way to do it, but is there something even worse that I am missing?)

long StringToLong( String s )
  {
      for ( int j = 0 ; j < s.length() ; j++ )
      {
          if ( s.chatAt(0) == '0' )
          {
              s = s.substring(1);
          }
          else
          {
              break;
          }
      }
      return Long.parseLong( s );
  }

No I don't know if it does always work because you have this silly "j" that is incrementing as the string is getting shorter. Now if the string is

"000011"

then j will meet the length of the string with only half of the zeros removed, and then you'll end up with Long.parseLong( "011" ) which may be parsed as Octal thus 9 instead of 11 which is what I think you want it to be.

Earl Purple · 2007-01-23 Reply Admin

Removing leading zeros from a string in C++:

s.erase( 0, s.find_first_not_of( '0' ) );

or

s = s.substr( s.find_first_not_of( '0' ) );

Note that if all of the characters are '0' it will make the final string empty. It might be that you want to keep just one of them.

Converting string to long:

std::istringstream iss( s );
long lv = 0;
iss >> lv;

(you can throw if iss >> lv returns an error and can also check if there is anything left over and throw if that is the case as well).

masklinn · 2007-01-23 Reply Admin

verisimilidude:
  for ( int i = 1 ; i <= 10 ; i++ )
  {
      /* do something important */
      if ( condition ) i = 11;
      /* do something else important */
  }
should be refactored into:
  bool lastLoop = false;
  for ( int i = 1 ; i <= 10 && !lastLoop ; i++ )
  {
      /* do something important */
      bool lastLoop = ( condition );
      /* do something else important */
  }
so that we don't rely upon 'magic' properties of the loop variable. Yes, it takes an extra byte, but it is so much clearer.

Oh god, yet another trainwreck.

You know what? break exists, use it. your code could be rewritten as the clearer, nicer and smaller-footprint following:

for(int i=1; i<=10; ++i) {
    /* do something important */
    /* do something else important */
    if(condition) { break; }
}

As mentioned by others, though, this only works if do something else important doesn't use the value of i

chrismcb:
I hope you don't go and build a new factory every time you seem something strange. (who cam up with the term "refactor" anyways?)

To the second question, it seems that was born from a conversation between William (Bill) Opdyke, author of the first paper (his PhD thesis) on refactoring, and Ralph Johnson (of GoF fame). It's first official use was a SOOPPA paper by Opdyke and Johnson in September 1990

For more informations about the term's ethymology: http://martinfowler.com/bliki/EtymologyOfRefactoring.html

And refactoring is nowadays considered to be in relation with mathematics' factorization notion, not "factory building".

It also seems that the word factoring has been used in the Forth community since the 80's, with the meaning later given to one of Refactoring's methods (Extract Method, as factoring in the Forth community means "breaking a word [Forth name for functions] into smaller pieces"

Alchymist · 2007-01-23 Reply Admin

Break is easy. This is from an application I work with

for (i=0; i < SOME_BIG_NUMBER; i++) { if (i == 0) { /* Do something */ } else { break; } }

Edit: OK, so how do I persuade the forum that indentation is a good idea in code?

rmr · 2007-01-23 Reply Admin

chrismcb:
who cam up with the term "refactor" anyways?)

Please read (don't skim) the first two chapters of this book: http://www.amazon.com/exec/obidos/ASIN/0201485672

2007-01-23 Reply Admin

The first loop probably looks that way because the Enterprise Architect said "Never use break, it's a poorly disguised GOTO. Any language with break is totally broken."

2007-01-23 Reply Admin

Trondster:
However, when parsing numbers in JavaScript, leading zeros indeed signify octal numbers. That's why many poorly written date selectors fail for august and september..

That isn't quite correct, either. In JavaScript, there is a second argument to parseInt() which specifies the radix. It is only when the radix argument is 0 (or null, or undefined -- which it will be if you didn't pass it in) that a leading 0 or 0x signifies octal or hexadecimal.

parseInt("0177") == 127 parseInt("0177", 10) == 177

Meanwhile, in Java Long.parseLong() and Integer.parseInt() can handle leading zeroes and assume the base is 10. If you want the magic handling of 0 and 0x you have to use Integer.decode() or Long.decode().

2007-01-23 Reply Admin

gwenhwyfaer:
Bunny:
Curious, what would be a better way to do it?

Your comment makes me very sad.

(...But not as sad as the reply page. I use links-hacked, and I'm typing this reply in a 2x20 textbox.

Ouch.)

It was an earnest question. Why would it make you sad? I'm just trying to learn here.

2007-01-23 Reply Admin

Try using the BBCode [code] tag...

for (i=0; i < SOME_BIG_NUMBER; i++) 
{ 
  if (i == 0) 
  { 
    /* Do something */ 
  } 
  else 
  { 
    break; 
  } 
}

brendan · 2007-01-24 Reply Admin

masklinn:
You know what? `break` exists, use it. your code could be rewritten as the clearer, nicer and smaller-footprint following:
for(int i=1; i<=10; ++i) {
    /* do something important */
    /* do something else important */
    if(condition) { break; }
}

that only works if condition is still true at the end of the loop, what happens if the condition result changes (If the result didn't change then you could use a break). You have to resort to the solution posted by verisimilidude.

Joery · 2007-01-24 Reply Admin

The third snippet is a perfect example of why you should ALWAYS use braces, even if the language does not require them.

2007-01-25 Reply Admin

Even though it is bad to modify a loop iterator to exit a loop, it is worth noting that the break statement hasn't been with us that long. Granted though, a while loop is a better structure to use in place of a lot of for loops that I see.

Please, please, all of you out there, don't do anything except for count and iterate over one variable in a for loop.

No more of this kind of nonsense... for (j=2; x < y && z > q; t++) Even programming books have a retarded variant of this. If you think about using the for loop as a shortcut, just stop right there and write it in the 8 lines that it deserves.

Thanks. Sorry for the rant. I've just seen this kind of dumb code on every project that I have to port.

Saladin · 2007-01-26 Reply Admin

This far in and only ONE Spinal Tap reference that I have seen so far?

For shame.

2007-01-26 Reply Admin

Actually not all the zeros are removed.

The string gets shorter however j get larger. Consider "000000" When j is 3, s is "000" so the loop stops at this point. Only half the string will be scanned.

So this works so long at the number zeros is half the length of the string or less.

Now what does it do with "0", it turns it into "" and then throws an exception.

I assume the use of "chatAt" should be charAt

2007-01-29 Reply Admin

I'm surprised no one mentioned that the first bit of code is an example of "structured programming", which as all good dobees will remember, allowed only one exit from a loop.

At a minimum it needed a comment, though. And why the explicit numbers? C++ allows #defines.

2007-01-29 Reply Admin

for ( int i = 1 ; i <= 10 ; i++ ) { /* do something important / if ( condition ) i = 11; / do something else important */ } I agree with chrismcb that this code is no wtf (it's not even near being one)

Also i'd like to point out that all of the "optimizations" posted for the code above (until now) could break it.

what if the code looks sth. like this:

  for( int i = 1 ; i <= 10; i++ ){
      ...
      if (condition) i = 11;
      x += f(i);        
       // i == 11 -> Blah Blah Blah
      if (i == 11){
         z += abs(x/2);
         ...
      }
      ...
  }

if (z < 0) StartNuclearWar();

So changing the code without knowing what /* do something else important */ does could not only break it but also start a thermonuclear War.

2007-11-09 Reply Admin

The following will only work for positive numbers, wouldn't be too hard to change it though...

public static long ZeroPaddedStringToLong(string s) {
  try {
    for (int i=0; i<s.Length; i++)
      if (s[i] != '0')
        return long.Parse(s.substring(i));
  } catch(Exception) {}
  return 0;
}

public static string ZeroPadLong(long l, int totalLength) {
  if (l < 0) 
    throw new ArgumentException(
      "Input number less than xero."
    );
  StringBuilder sb = new StringBuilder(totalLength);
  for (int i=0; i<totalLength-tmp.Length; i++)
    sb.Append('0');
  sb.Append(tmp);
  return sb.ToString();
}
</pre>

2007-11-09 Reply Admin

Correction....

Michael J. Ryan:

public static string ZeroPadLong(long number, 
  int totalLength) 
{
  if (l < 0) 
    throw new ArgumentException(
      "Input number less than xero."
    );
  string tmp = number.ToString();
  StringBuilder sb = new StringBuilder(totalLength);
  for (int i=0; i<totalLength-tmp.Length; i++)
    sb.Append('0');
  sb.Append(tmp);
  return sb.ToString();
}
</pre>

Breaking Out of the Box

Leave a comment on “Breaking Out of the Box”