Thanks a heap for the response. I truely appreciate it and yes I should have clarified overall debugging and style tips was what I was after.

I'm not sure if you just want to have this code debugged, or if you are asking for some style tips too.

Anyway here's some advice to point you in a better direction:

• Don't worry about "efficiency". Focus firstly on readibility and functionality.

• You seem to be going to a lot of effort to get to that single return b at the bottom of the method. Single return points are nice but should not be at the expense of simplicity or readibilty. So you can simplify some of that b logic into oblivion.

• I don't recommend "magic" numbers in code. The next person to read this won't necessarily know what you meant. e.g. Instead of 65 and 90 why not use 'A' and 'Z'?

• You need to consider what are all possible inputs. The Kata description implies the string could be null so you need to cope with that instead of doing toUpperCase without 1st checking for null. In fact, THIS IS YOUR BUG. If you deal properly with the null input then your code passes OK.

• If in doubt what is going on you then you ought to scatter some debugging output around in your code - eg if you did System.out.println to show what the input strings s1 and s2 were then you would have found the problem.

• When something gives a stacktrace read it! NPE on your line 6. Can't be any clearer than that :-)

java.lang.NullPointerException
	at Kata.compare(Kata.java:6)
	at KataTests.BasicTests(KataTests.java:15)
	at ...

• All the comments were obviously well intended, but in this case less comments and better variable names would be an improvement. For example, instead of b why not call it retValue - then the meaning is clear and the comment is not needed anymore. Also comments that just say exactly what the code is doing serve no purpose: else if(sum1==sum2) b = true; // else compare sums.

Hope some of that helps

Cheers,
DM