Awesome use of control characters to clear the terminal and reset the cursor postion. I was never doing that stuff in these classes. My programs always just printed out stuff over and over and over. So, pro stuff right there!
Bring the main
method up so it is the first method to appear in the file.
Make more methods to encapsulate different parts of the main method. Think of the main method more as a table of contents than paragraphs of code explaining the program.
I'd aim for something like this:
public static void main(String[] args) throws InterruptedException {
welcomeMessage();
int choice = promptForSize();
ponderChoice(choice);
printColumnCombos();
printGame();
printResults(moves);
}
- Global variables (str, moveWords, col1, col2, col3, moves) should be in ALL_UPPERCASE (also, you could probably localize their scope in the main method and pass them as parameters, but whatever.)
- Prefer
ArrayList<String>
overArrayList< String >
(no spaces). - Prefer
fromRod
overfrom_rod
(camelCase, not snake_case).
There's a weird thing I wonder if they've talked about in class that's "interface vs implementation." You should use interfaces to declare the type of things and use implementations to actually create things.
Basically, prefer List
over ArrayList
or LinkedList
on the left where you
declare the type. The reason for this is the two implementations ArrayList
and LinkedList
are both implementations of the List
interface. If you
declare them as the more general List
you can use them in more places.
It is definitely a "rectangle vs square" situation.
private static List<Integer> col3 = new ArrayList<Integer>();
private static List<Integer> col4 = new LinkedList<Integer>();
Being able to use them in more places might make more sense if we're talking
about declaring the types of parameters for a method. If you declare the
parameter as having type List
you can pass in either ArrayList
or
LinkedList
. If you delcare a method as just ArrayList
you couldn't pass
in a LinkedList
.
// This declaration is more broad
private static someMethod(List<Integer> list) {}
// This type declaration is more restrictive
private static someMethod(ArrayList<Integer> list) {}
Never abbreviate words. You write ind
instead of index
in a few places.
Abbreviations don't really saving anything. They tend to contribute to
confusion and inconsistency. (OK I'm also a hypocrite here because I think
"col" for "column" is alright.)
Here's a personal preference from me. Prefer longer variable names. Instead of
n = scan.nextInt()
consider choice = scan.nextInt()
. Longer variable names
are easier to search for when you're trying to find where else they occur in
code.
Single variable names I personally find especially hard to search for. When I
write any programs with x
and y
coordinates and weird tick I've developed
is always writing them as xx
and yy
so I can search for them easier and the
letters "x" and "y" in other variable names don't barge in my search results.
Make more utility methods!
You have these lines of code repeating several times. Make a method called
something like refresh
or something even cooler like cursorBlit()
so
you can call that method instead of writing this code redundantly everywhere.
System.out.print("\033[2J"); // clear terminal
System.out.print("\033[0;0H"); // place cursor at top left corner
Similarly you can reduce redundancy in the structure of printing these messages.
Make a method called display(String message)
or something.
printGame(n); // print first state of the game before computation
System.out.println();
System.out.println("Hmm, let me think here.");
// ... other code ...
printGame(n); // rerprint first state of the game
System.out.println();
System.out.println("Ohh, I know! here look:");
private static void display(String message) {
printGame(n);
System.out.println();
System.out.println(message);
}
You have a lot of for loops that exist just to print out a character a certain amount of times. Prime canidate to make a method for that.
private static void repeatChar(String text, int times) {
for (int i = 0; i < times; i++) {
System.out.print(" ");
}
}
int ind = col1.get(i);
for (int y = 0; y < n - ind; y++) {
System.out.print(" ");
}
for (int y = 0; y < ind; y++) {
System.out.print("-");
}
System.out.print(ind);
if (ind > 9) {
for (int y = 0; y < ind - 1; y++) {
System.out.print("-");
}
} else {
for (int y = 0; y < ind; y++) {
System.out.print("-");
}
}
for (int y = 0; y < n - ind; y++) {
System.out.print(" ");
}
int ind = col1.get(i);
repeatChar(" ", n - ind);
repeatChar("-", ind);
System.out.print(ind);
if (ind > 9) {
repeatChar("-", ind - 1);
} else {
repeatChar("-", ind);
}
repeatChar(" ", n - ind);
You repeat this structure of code several times. You can parameterize it.
// ...
} else if (str.get(i).charAt(1) == 'A' && str.get(i).charAt(2) == 'C') {
int disc = col1.get(col1.size() - 1);
int index = col1.size() - 1;
col1.remove(index);
col3.add(disc);
} else if (str.get(i).charAt(1) == 'B' && str.get(i).charAt(2) == 'C') {
int disc = col2.get(col2.size() - 1);
int index = col2.size() - 1;
col2.remove(index);
col3.add(disc);
// ...
columnCombo('A', 'C', col1, col3);
columnCombo('B', 'C', col2, col3);
// ...
private static columnCombo(Character char1, Character char2,
List<Integer> addColumn, List<Integer> removeColumn) {
if (str.get(i).charAt(1) == char1 && str.get(i).charAt(2) == char2) {
int disc = removeColumn.get(removeColumn.size() - 1);
int index = removeColumn.size() - 1;
removeColumn.remove(index);
addColumn.add(disc);
}
}