Coding Horror
Given the rate that I’m running accross these little horrors, this may become a regular post.
public static String trimLeadingZeros( String trimString )
{
String retVal = trimString;
boolean nonZeroFound = false;
StringBuffer rtnString = new StringBuffer();
if ( trimString != null ) {
for ( int index = 0; index < trimString.length(); index++ ) {
char c = trimString.charAt( index );
if ( c == '0' && nonZeroFound == false ) {
// skip all leading zeros
} else {
nonZeroFound = true;
rtnString.append( c );
}
}
}
if ( rtnString.length() > 0 ) {
retVal = rtnString.toString();
}
return retVal;
}
Obviously this person doesn’t have a degree in computer science or they failed out.
- The have an empty if statement, what you’re not capable of doing the necessary boolean algebra in your head to invert the conditional, give me a break
- Hey lets scan every character whether you need to or not, making this O(n) where n is the total number of charaters in all strings being scanned (sheesh)
- How many damn variables do you need to do this 5! Could we make this any more complex
- The real shame is that the JDK doesn’t have a general strip/trim function, they have a trim whitespace, so why not generalize it.
So in about 5 minutes I put a more general version together which O(f(x)) << O(n), in the general case my version is 2.5 times faster than the horror. For the pathalogical case my versio is only.8 time faster, and for the ideal case we’re 3 times faster. This was all done over 1 million iterations using the same string that was 20 characters long.
public static String trimLeadingChar( String trimString, char trimChar )
{
try {
String tempString = trimString;
int offset = 0;
while( tempString.indexOf( trimChar, offset ) == offset )
offset++;
tempString = tempString.substring( offset );
if( tempString.length() == offset )
return trimString;
return tempString;
} catch( NullPointerException ex ) {
return trimString;
}
}
I’m using a very Pythonic idom here, which is to not care about the NullPointerException, so instead of explicitly checking
for trimString == null, we just catch the excetption and do no harm. I could have made the while loop empty, but frakly I find
that a little bit vulgar.
Why not just use a regular expression? Excuse the pseudo style, it’s been a while:
}I hadn’t taken that approach, because I generally thought the regex would be much slower than the more traditional options. I’ve tried roughly 6 different approaches that I came up with trying different weird things, I’ll give this one a try as well, since I’ve already gone way over board on this one. Interestingly enough, the fastest approach I’ve found (which I didn’t write) was to copy the string into a char array and then index through the array, what does this say about Java :-(, I’ll update the post with results from both of these approaches.
Hey! Stop laughing at my code….
But seriously, I would have probably put the null check as the first line in the method, something like:
if ( trimString == null ) return null;Fowler calls it a guard clause and for the people who are not afraid to use more then one return statement in the method it is quite effective in reducing the complexity.
For the actual trimming, I would have simply used the library Integer.parseInt or Double.parseDouble to take care of the details. Yes, at that point it is no longer trim method but it might be justified if the returned value is used as a number later on in the code.
In fact, I’d be really looking for usages of this trim method to convice myself that the method should not have been a parse method in the first place.
In case, this has to manipulate strings and only trim the leading zeros, advancing offset seems the most appropriate.
Regex reduces the number of lines but it adds another skill requirement for the maintainer.
Regular expressions on most platforms are very fast; I can’t speak for Java. The pattern is precompiled so there’s very little work beyond what is essential. If you want to benchmark…
Ideally, maintainers will never look at the regex code since it should work 😉 I’m laughing at myself now because I realize you want to keep at least one 0 so the pattern has to look something like “^0*(.*.)$” -> “\1” which guarentees at least one char in the result.