Problem
This code that I wrote for a lab works perfectly and gives the desired output. That said, it’s over 200 lines of code to reach the result. This seems ridiculous, even when you consider that there is a restriction to only using the for loop to do it.
What are some ways I could have written this program better? Have I done anything badly? Can I improve in any way or is the program written in the best way possible given the restrictions?
public class lab2 {
public static void main(String[] args) {
banner();
System.out.println();
banner2();
System.out.println();
banner3();
}
static void banner() {
for (int a = 1; a <= 7; a++) {
StringBuilder all = new StringBuilder();
for (int b = 6; b >= a; b--) {
all.append("*");
}
for (int b = 1; b <= a; b++) {
all.append(" ");
}
for (int b = 6; b >= a; b--) {
all.append("//");
}
for (int b = 2; b <= a; b++) {
all.append("\\");
}
for (int b = 1; b <= a; b++) {
all.append(" ");
}
for (int b = 6; b >= a; b--) {
all.append("*");
}
System.out.println(all.toString());
}
}
static void banner2() {
StringBuilder outs = new StringBuilder();
outs.append("+");
for (int h = 1; h <= 6; h++) {
outs.append("-");
}
outs.append("+");
System.out.println(outs.toString());
for (int b = 2; b <= 4; b += 2) {
for (int a = 1; a <= 3; a++) {
StringBuilder all = new StringBuilder();
for (int c = 1; c <= 1; c++) {
all.append("|");
}
for (int c = 2; c >= a; c--) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("^");
}
for (int c = 2; c <= a; c++) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("^");
}
for (int c = 2; c >= a; c--) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("|");
}
System.out.println(all.toString());
}
}
System.out.println(outs.toString());
for (int b = 2; b <= 4; b += 2) {
for (int a = 1; a <= 3; a++) {
StringBuilder all = new StringBuilder();
for (int c = 1; c <= 1; c++) {
all.append("|");
}
for (int c = 2; c <= a; c++) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("v");
}
for (int c = 2; c >= a; c--) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("v");
}
for (int c = 2; c <= a; c++) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("|");
}
System.out.println(all.toString());
}
}
System.out.println(outs.toString());
}
static void banner3() {
StringBuilder outs = new StringBuilder();
outs.append("+");
for (int h = 1; h <= 9; h++) {
outs.append("-");
}
outs.append("+");
System.out.println(outs.toString());
for (int a = 1; a <= 4; a++) {
StringBuilder all = new StringBuilder();
for (int c = 1; c <= 1; c++) {
all.append("|");
}
for (int b = 4; b >= a; b--) {
all.append(" ");
}
for (int b = 2; b <= a; b++) {
all.append("/");
}
for (int b = 3; b >= 3; b--) {
all.append("*");
}
for (int b = 2; b <= a; b++) {
all.append("\");
}
for (int b = 4; b >= a; b--) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("|");
}
System.out.println(all.toString());
}
for (int a = 1; a <= 4; a++) {
StringBuilder all = new StringBuilder();
for (int c = 1; c <= 1; c++) {
all.append("|");
}
for (int b = 1; b <= a; b++) {
all.append(" ");
}
for (int b = 3; b >= a; b--) {
all.append("\");
}
for (int b = 3; b >= 3; b--) {
all.append("*");
}
for (int b = 3; b >= a; b--) {
all.append("/");
}
for (int b = 1; b <= a; b++) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("|");
}
System.out.println(all.toString());
}
System.out.println(outs.toString());
for (int a = 1; a <= 4; a++) {
StringBuilder all = new StringBuilder();
for (int c = 1; c <= 1; c++) {
all.append("|");
}
for (int b = 1; b <= a; b++) {
all.append(" ");
}
for (int b = 3; b >= a; b--) {
all.append("\");
}
for (int b = 3; b >= 3; b--) {
all.append("*");
}
for (int b = 3; b >= a; b--) {
all.append("/");
}
for (int b = 1; b <= a; b++) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("|");
}
System.out.println(all.toString());
}
for (int a = 1; a <= 4; a++) {
StringBuilder all = new StringBuilder();
for (int c = 1; c <= 1; c++) {
all.append("|");
}
for (int b = 4; b >= a; b--) {
all.append(" ");
}
for (int b = 2; b <= a; b++) {
all.append("/");
}
for (int b = 3; b >= 3; b--) {
all.append("*");
}
for (int b = 2; b <= a; b++) {
all.append("\");
}
for (int b = 4; b >= a; b--) {
all.append(" ");
}
for (int c = 1; c <= 1; c++) {
all.append("|");
}
System.out.println(all.toString());
}
System.out.println(outs.toString());
}
}
Solution
I hope there is no restriction to create your own method. Here is how I refactor your banner() method:
static void appendChars(StringBuilder sb, char c, int count) {
for(int i = 0; i < count; i++) {
sb.append(c);
}
}
static void banner() {
for (int a = 1; a <= 7; a++) {
StringBuilder all = new StringBuilder();
int numOfAsterisks = 7 - a;
int numOfSpaces = 7 - numOfAsterisks;
int numOfSlashes = (7 - a) * 2;
int numOfBackslashes = (a - 1) * 2;
appendChars(all, '*', numOfAsterisks);
appendChars(all, ' ', numOfSpaces);
appendChars(all, '/', numOfSlashes);
appendChars(all, '\', numOfBackslashes);
appendChars(all, ' ', numOfSpaces);
appendChars(all, '*', numOfAsterisks);
System.out.println(all.toString());
}
}
Noted that best code in real life is the code that works and be readable at the same time. It is quite noticeable that by extracting for loop into its own method, it makes banner() a lot more readable.
The other thing is how the code calculates everything before output, this makes it a lot easier to debug and also providing hints about how you calculate number of symbols to output.
After all, do not fall into the thought that shorter code is better. You can reduce your line number by using for loop like for(int i = 0; i < 9; i++) { } without new line, but that makes no merits at all. Shorter code is better only when it improves the code quality, which mean longer code can also be better too.