# Decoding binary string message

I am trying to learn some Java on my own, and I am tackling some "programming challenge" problems to practice the little I have learnt so far.

Could anybody constructively critique this beginner's effort (source on Bitbucket)?

public class BinaryCode {
public String[] decode(String message) {
int messageLength = message.length();
return new String[] { decodeMessage(message, messageLength, 0),
decodeMessage(message, messageLength, 1) };
}
private String decodeMessage(String encodedMessage, int messageLength,
int assumedFirstDigit) {
StringBuilder decodedMessage = new StringBuilder(messageLength);
int nextDecodedDigit = assumedFirstDigit;
for (int i = 0; i < messageLength; i++) {
if (nextDecodedDigit != 0 && nextDecodedDigit != 1) {
return "NONE";
}
decodedMessage.append(nextDecodedDigit);
int encodedDigit = encodedMessage.charAt(i) - '0';
int decodedDigit = decodedMessage.charAt(i) - '0';
nextDecodedDigit = encodedDigit - decodedDigit;
if (i > 0) {
nextDecodedDigit -= decodedMessage.charAt(i - 1) - '0';
}
}
int checkValue = decodedMessage.charAt(messageLength - 1) - '0';
if (messageLength > 1) {
checkValue += decodedMessage.charAt(messageLength - 2) - '0';
}
if (checkValue == encodedMessage.charAt(messageLength - 1) - '0') {
return decodedMessage.toString();
}
return "NONE";
}
}

-

These critiques are mostly style and syntax related, and do not address your algorithm (you could probably find the simplest possible algorithm with Google anyway).

• make decode and decodeMessage static
• explicitly hide the constructor for BinaryCode, e.g.

// hide constructor
private BinaryCode() {}

• remove unneseccary - 0 from encodedDigit and decodedDigit declarations
• let decodeMessage calculate messageLength (fewer parameters are nearly always better)
• avoid using magic numbers and string literals (-1, -2, "NONE", etc...)
• create method lastChar and call it instead of messageLength - 1 e.g. int checkValue = lastChar(decodedMessage) - '0';

private static char lastChar(CharSequence sequence) {
int lastIndex = sequence.length() - 1;
return sequence.charAt(lastIndex);
}

• replace decodedMessage.charAt(messageLength - 2) and decodedMessage.charAt(i - 1) with previouslyDecodedChar e.g.

char previouslyDecodedChar = '\0';
for (int i = 0; i < messageLength; i++) {
// ...
if (i > 0) {
nextDecodedDigit -= previouslyDecodedChar - '0';
}
previouslyDecodedChar = decodedMessage.charAt(i);
}
int checkValue = lastChar(decodedMessage) - '0';
if (messageLength > 1) {
checkValue += previouslyDecodedChar - '0';
}

-
Thanks very much for your comments. I really appreciated them. - '0' is not redundant. The method signatures were mandated by the exercise, and I "think" the instantiability is too, so I could not hide the ctor. – Robottinosino Nov 8 '12 at 4:31