Many amateur and professional software developers take on the attitude "if it ain't broke, don't fix it". In other words, they will write their code until it works and then they are done with it. After all, isn't the point to have working software? Tight company deadlines encourage this approach as well, as the perception is that there isn't time to refactor and clean the code. It is my position that even with tight deadlines, you don't have time not to clean your code. In the long run, just getting the code working and then moving on is going to come back to bite you.
Think about it. You work long and hard to get a new feature working. Once it's working, you deliver it and move on to the next feature. A few weeks later, the initial feature needs to be enhanced. Will you be the one to work on it? Maybe. If not, then pity the poor soul who has to untangle what took you days to tangle up in the first place. If so, then just hope that you can remember what it was that you were doing, because it's unlikely to be clear from the code.
Let's take a look at a simple example.
[pre class="brush:java" title="GamePlayer.java"]import java.util.Scanner;
public class GamePlayer {
private int errors = 0;
private String guesses = "";
private String partialSolution;
public String takeTurn(char guess, String target) {
if (partialSolution == null) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < target.length(); i++) {
builder.append('*');
}
partialSolution = builder.toString();
}
if (!Character.isAlphabetic(guess)) {
return "Error: you can only guess letters.";
}
if (guesses.indexOf(Character.toLowerCase(guess)) >= 0) {
return "You already guessed that";
}
guesses += Character.toLowerCase(guess);
if (target.toLowerCase().indexOf(Character.toLowerCase(guess)) == -1) {
errors++;
}
int count = 0;
for(int i = 0; i < target.length(); i++) {
if (target.toLowerCase().charAt(i) == Character.toLowerCase(guess)) {
count++;
partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
}
}
String verb;
String pluralModifier;
String number;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
String output = String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
if (errors > 5) {
output += "\nThe word is " + target + "\nYou lose.";
} else if (partialSolution.indexOf('*') < 0) {
output += "\nThe word is " + partialSolution + "\nYou win!";
} else {
output += "\n" + partialSolution;
}
return output;
}
}[/pre]
If, at this point, you find yourself saying, "He said a simple example. This doesn't look simple," then I have made my point. This really is a simple example. It is functionally and syntactically correct. Please answer the following questions for yourself and pay attention to how long it takes to be able to do so.
- At a high level, what does this code do?
- How many errors can be made before losing?
- What are lines 36-52 doing?
- What is the point of the instance variable partialSolution?
- Suppose that the rules of the game are to be changed so that if the solution contains multiple copies of the same letter then they are filled in one at a time when that letter is guessed. For example, if the word was 'feelers' then the first guess of the letter 'e' would reveal the first 'e', but not the others (*e*****), the second guess of the letter 'e' would reveal the second 'e' (*ee****), and so on.
- Where would you start?
- How many points in the code would need to be touched?
- What if the new rule was to be optional, chosen by the player before starting the game?
The point I am making here is that the above questions are not easy to answer quickly. The code works, but it is not obvious what it does or what all its parts are for. This is for a fairly simple game written by one person in an hour or so. The problem gets exponentially worse for complex business domains written and maintained by multiple people on multiple teams over the course of years. This, my friends, is where spaghetti code comes from.
The code above is not only dirty, but it is also not object oriented. (Just because it's Java doesn't mean it's object oriented.) It is structured in that it doesn't have goto statements of any type (goto, continue, break), but it is very procedural. In part 2 of this series, I will start with this code and discuss how to make it cleaner and more object oriented.
The code above is not only dirty, but it is also not object oriented. (Just because it's Java doesn't mean it's object oriented.) It is structured in that it doesn't have goto statements of any type (goto, continue, break), but it is very procedural. In part 2 of this series, I will start with this code and discuss how to make it cleaner and more object oriented.
