Different character outputs using loops and strings

Question

I've just given this as a response to an absolute beginner on SO, and I'm wondering how terrible it is.

Output for an odd number of lines:

>
>>>
>>>>>
>>>
>

Output for an even number of lines:

>
>>>
>>>>>
>>>>>
>>>
> 

The code:

int lines = 7;
int half = lines/2;
String carat = ">";
int i;
//better to declare first or does it not matter? 
if(lines%2==0){boolean even = true;}
else{boolean even = false;}
for(i=1;i<=lines;i++){
 System.out.println(carat + "\n");
 if(i==half && even){System.out.println(carat+"\n");}
 if(((i>=half && even) || (i>=half+1)) && i!=lines){
 carat = carat.substring(0,carat.length()-2);
 }else{
 carat = carat + ">>";
 }
}

This code just seems so ugly, not considering the fact that there is no method that takes number of lines and that it is abusing integer rounding.

Answer 1

better to declare first or does it not matter?

No. These days most people believe variables should be declared as late as possible, right by where they are used.

This code just seems so ugly.

It is. A little whitespace in there would help a lot. Which is easier to read:

// This:
if(i==half && even){System.out.println(carat+"\n");}
// Or this:
if (i == half && even) {
 System.out.println(carat+"\n");
}

that it is abusing integer rounding.

There's no abuse, and it's not rounding. Truncation on integer arithmetic is a specified part of the language. Feel free to use it. In this case, it does exactly what you want.

If I were writing this, I'd do:

printCarats(int numLines) { 
 for (int line = 0; line < numLines; line++) {
 // Mirror vertically across center line.
 int halfLine = line;
 if (line > numLines / 2) halfLine = numLines - line - 1;
 int numCarats = 1 + halfLine * 2;
 for (int i = 0; i < numCarats; i++) {
 System.out.print(">");
 }
 System.out.println();
 }
}

• Bert F: I would rephrase "should be declared as late as possible" rather as "should be declared in as narrow a scope as possible". Perhaps the meaning is the same to most people, but to me, the talking about scope is clearer. For example, the benefit of declaring int i in the for loop statement is that its scope is limited to the loop statement and block.

Answer 2

 if(lines%2==0){boolean even = true;} 
 else{boolean even = false;} 

Does this work for you? First, it looks like this makes the even variable only have scope in the decision block. You shouldn't be able to reference this further down in the code if you are declaring it in the braces.

Also, you can probably get rid of the if/else by just using the logic test to define the even variable:

boolean even = (lines%2 == 0); //parens for clarity

Answer 3

Know your libraries, use your libraries...

int lines = 7;
for (int i = 0; i < lines; i++) {
 int k = Math.min(i, lines-i-1);
 char[] line = new char[2*k+1];
 java.util.Arrays.fill(line, '>');
 System.out.println(new String(line));
}

• barjak: nice, except it's java.util.Arrays
• Arne: I really like this. But it could be even better - you should initialize a String of length (lines + 1) / 2 before the for loop and print it with substring(...) inside.

Answer 4

Since there are not any requirements listed for this simple program, except some output that is lacking detail and since this was given to a beginner, I would start with a template like the one included and start the new programmer off with good practices like, code indentation, comments use of CONSTANTS etc.

 /*
 * this program will print the > greater than sign when the loop index is even
 * and the < less than sign when the loop index is odd
 * the program will test up to the integer value 7
 * 
 * programmer name
 * date
 */
//begin class definition
public final class PrintStrings {
 //required main method
 public static void main(String[] args){
 String ODDCARAT = "<"; // ODDCARAT constant
 String EVENCARAT = ">";// EVENCARAT constant
 int i;
 //loop through 7
 for(i=1;i<=7;i++){
 //test if integer is odd or even by % by 2 and checking for remainder
 if(i %2 == 0){
 System.out.println(EVENCARAT);
 }else{
 System.out.println(ODDCARAT);
 }//end if
 }//end for
 }//end main
}//end class