Bug - Fixed Compile errors preventing KoLmafia from compiling with JDK 1.4

In light of Veracity's notes that KoLmafia has code formatting standards I was unaware of in my other thread about code cleanup... I won't be formatting the code in my future patches.

Anyway, that being said, KoLmafia as it stands now has several compile errors that cause compilation with JDK 1.4 to fail (it silently succeeds with JDK 5 or newer, though - hence why this problem wasn't spotted earlier).

Attached is a small patch to fix the 3 problems:

There is no "StringBuilder" in Java 1.4 (solution: Replace with StringBuffer)
String.replace() expects two char arguments, not two Strings (solution: use String.replaceAll() instead)
Integer.valueOf(int) doesn't work in Java 1.4 (solution: since this is only called once with a constant, and Integer.valueOf(String) DOES exist, surround the constant with quotes)
 

fronobulax

Developer
Staff member
Questions - since the official standard is to run with a Java 1.4 JRE are these really necessary? I think almost everyone is building with a JDK that is newer than 1.4 but using the target option to build for 1.4. It seems to me that either it is not necessary for this code to be at 1.4 or the target option is not doing what people expect it to do (and no one has complained).

If a decision is made to change the standard to 1.5, can this safely be ignored?

These questions are not to belittle your work, which is appreciated, but to save me from the exercise of installing a 1.4 JRE to answer them myself.
 

Veracity

Developer
Staff member
Integer.valueOf( 0 ) is dumb - but the solution is not to make it into Integer.valueOf( "0" ).
How about new Integer( 0 )? :)

I'll look at the rest of this later.
 
Questions - since the official standard is to run with a Java 1.4 JRE are these really necessary? I think almost everyone is building with a JDK that is newer than 1.4 but using the target option to build for 1.4. It seems to me that either it is not necessary for this code to be at 1.4 or the target option is not doing what people expect it to do (and no one has complained).

If a decision is made to change the standard to 1.5, can this safely be ignored?

These questions are not to belittle your work, which is appreciated, but to save me from the exercise of installing a 1.4 JRE to answer them myself.
Actually, the build.xml file specifies the source 1.4 option, but does NOT specify the target option. This means that the resulting JAR does, in fact, require JRE 5, since that's the JDK used to build it.
 

fronobulax

Developer
Staff member
Thanks. I'm building with NetBeans on a Windows box and I know that my build process is not exactly the same as the one with Eclipse on a Mac (or ant from a command line). I theory I was setting a target of 1.4 but it is a moot point since I am not building releases.

Am I correct in inferring from your observations that the build process (as driven by build.xml) is neither enforcing a 1.4 code standard nor producing a jar that will run on 1.4?

Thanks.
 
Thanks. I'm building with NetBeans on a Windows box and I know that my build process is not exactly the same as the one with Eclipse on a Mac (or ant from a command line). I theory I was setting a target of 1.4 but it is a moot point since I am not building releases.

Am I correct in inferring from your observations that the build process (as driven by build.xml) is neither enforcing a 1.4 code standard nor producing a jar that will run on 1.4?

Thanks.
Actually, scratch that. It does in fact run on Java 1.4 (installed JDK 1.4u19 in Windows XP Mode on Windows 7 precisely to test this). However, if you set a 1.4 JDK as your active JDK for the KoLmafia project, and try compiling, you get errors. This li'l patch fixes those.
 

Theraze

Active member
Eh, maybe this is a FEATURE to compile on JDK 1.4 for a few weeks as the developers move to JRE 1.5 as the officially supported version but, as fronobulax pointed out, since the current 'standard' is JRE 1.4, and it works on that, the fact that it doesn't compile on JDK 1.4 isn't REALLY a bug, but more of a... transitional point. If it runs properly on JRE 1.4, it's working as designed.

Which is not to say that we can't clean up the code and figure out what exactly was being thought at various points, but... :)
 

Rinn

Developer
StringBuilders perform better then StringBuffers, which is why I used them.

Using string.ReplaceAll can lead to subtle bugs because it takes in a regular expression instead of a raw string to replace, unless that's the expected functionality in these cases which I don't believe it is.

Integer.ValueOf() should probably be changed to new Integer().

Regardless, aside from the Integer.ValueOf() I don't see any benefit to applying this patch since we're going to be using 1.5 pretty soon which would compile without issue.
 
Last edited:

roippi

Developer
When we switch to 1.5, we should be using Integer.valueOf instead of new Integer(), unless it's a situation where you explicitly need the wrapper. Integer.valueOf is faster. link
 

Rinn

Developer
Oh right, I think I saw that same article and that's why I used ValueOf in the first place.
 

Rinn

Developer
Okay that's fine but when we switch to 1.5 I'd like to change those back to StringBuilders since they do have better performance.
 

holatuwol

Developer
Yeah, we've got 315 (according to grep -R "new StringBuffer").

Most can switch, but Java never updated appendReplacement() to take in StringBuilder also, so there will need to be some massaging whenever Matchers are in play. Or the most frequently accessed code that uses StringBuffers (the relay browser request decoration stuff) will probably still wind up having to use a StringBuffer and so there's no real performance benefit in switching over.
 
Last edited:
Top