Printing a Hollow Butterfly pattern

Question

I am a beginner to Java and have studied up to loops only. After loops lecture above pattern was given to print as homework. I have successfully managed to print this pattern with following code:

public class Main {
    public static void main(String[] args) {
        int n = 5;
        for (int i = 1; i <= n; i++) {
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
            {
                System.out.print("*");
            }
            for(int j=1; j<=2*(n-i); j++){
                System.out.print(" ");
            }
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
            {
                System.out.print("*");
            }
            System.out.println(" ");
        }
        for (int i = n; i >= 1; i--) {
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
                System.out.print("*");
            for(int j=1; j<=2*(n-i); j++){
                System.out.print(" ");
            }
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
                System.out.print("*");
            System.out.println(" ");
        }
    }
}

I don't know if I have followed the best possible approach according to as much I have studied java until now. Please suggest if any improvements can be made.

Answer 1

I think your code is fine: it answers the question, and does not set any major red flag off. Good job!

There is one red flag, though: code repetition. The content of your two i-loops are identical. When that happens, you should put it inside a function, if you have learned how to do that.

There are a few ways to simplify your code.

First option: j tracks the index of the white space

I don't think this option is actually sensible, and I would go straight to the second one, but I'll describe it anyway in case you had not realize you could change the start index.

Here, each of your j-indexed loop counts from 1 to the number of whitespaces you want to include. You could change that and have j be the index of the character on the line. For example, for any line but the first and the last, the stars are positionned at indices (starting from 1 as you did rather than 0 as is usually done in Java and most computer languages) 1, i+1, 2*n-i+1, 2*n. Then, the whitespaces blocks have ranges [2,i], [i+2,2*n-i],[2*n-i+2,2*n-1]. The line for (int j = 1; j <= i - 2; j++){ could become for (int j = i+2; j <= 2*n-i; j++){.

It doesn't change much, and I would not go so far as to call that an improvement rather than an alternate solution.

Second option: only one loop per line

You obviously know how to use if-statements as well as loops, because you use four of them. You could leverage that to simplify the block you use to print a line: instead of breaking the j-loop, move the test inside.

I assume below that you also know how to use the if-else construct. If that is not the case, you can simply change the else line into another if-test.

System.out.print("*");
for (int j = 2; j < 2*n; j++){
    if (j == i || j == 2*n-i+1){
        System.out.print("*");
    } else {
        System.out.print(" ");
    }
System.out.println("*");
}

Third option: down to only two loops

In this option, I assume that you actually know about function calls and String concatenation. Starting from Java 11, you could replace the j-loops with a simple call to String::repeat. Then, you obtain something like System.out.println("*" + " ".repeat(...) + "*" + " ".repeat(...)" + ...) for all lines but the first and the last. I leave it to you to actually fill in the blanks if you are using Java 11.

Answer 2

You should be able to simplify this considerably.

I'd suggest using nested loops and putting a single if inside of these loops to determine which type of character to print, as opposed to a series of flat loops that handle each "run" to build a row in multiple steps as you're currently doing.

The "X" star locations are determined by the condition j == i || j == n - i - 1 where i is the current row index and j is the current column index.

For the first and last stars in each row, you can use the condition j == 0 || j == n - 1.

Combining the above four conditions gives a clear way to determine when to print a star or not for each cell.

For example:

public class Main {
    public static void printButterfly(int n) {
        n *= 2;

        for (int i = 0; i < n; i++) {
            for (int j = 0; j < n; j++) {
                if (j == 0 || j == i || j == n - i - 1 || j == n - 1) {
                    System.out.print("*");
                }
                else {
                    System.out.print(" ");
                }
            }

            System.out.println();
        }
    }

    public static void main(String[] args) {
        printButterfly(5);
    }
}

Answer 3

1. Any single-letter variable name is usually a mistake. Give variables meaningful names, even in simple exercises. I take it that i is line number and j is column number - why didn't you name them line and column? n is half the butterfly height/width, which doesn't have a good name, meaning it's probably not a good idea.
2. Look for patterns. Sticking with the 10*10 grid for now and sticking with Java style numbering such that lines and columns go from 0-9, we can see that you need to show asterisks at columns 0 and 9, plus asterisks at column (0+line) and column (9-line).

Your code can be simplified to something like the code below. I've thrown in a printLine() method to illustrate breaking the process down a little. It would not be difficult to generalise this for varying butterfly sizes, similarly to ggorien's example.

public class Butterfly {

  public static void main(String[] args) {
    for (int line = 0; line < 10; line++) {
      printLine(line);
    }
  }

  private static void printLine(int line) {
    for (int column = 0; column < 10; column++) {
      if ((column == 0) || (column == 9)                      // outside edges
          || (column == (0 + line)) || (column == (9 - line)) // inside edges
      ) {
        System.out.print('*');
      }
      else {
        System.out.print(' ');
      }
    }
    System.out.println();
  }

}

Answer 4

I have another suggestion: Instead of System.out.printing, you could build a single string containing all the ASCII art and print it once to console. To begin with, it would be simpler if you create a character matrix, "paint" the borders, paint the diagonals, and dump the matrix to a single string.

Why?

Mainly, because the calls to System.out.printX are expensive (see below).

Summa summarum

All in all, I had the following alternative rewrite in mind:

package com.yourcompany.fun;

import java.util.Arrays;

public final class ASCIIButterfly {
    
    private static final char ASTERISK = '*';
    private static final char NEW_LINE = '\n';
    private static final int DEFAULT_N = 5;
    private static final int MINIMUM_N = 1;
    
    public static String getButterflyArt(final int n) {
        return new CharacterButterflyMatrix(n).toString();
    }
    
    private static final class CharacterButterflyMatrix {
        
        private final char[][] matrix;

        public CharacterButterflyMatrix(final int n) {
            matrix = new char[2 * n][2 * n];
            setAllMatrixComponentsToEmptySpace();
            paintButterfly(n);
        }
        
        @Override
        public String toString() {
            return convertCharacterMatrixToString(matrix);
        }
        
        private void setAllMatrixComponentsToEmptySpace() {
            for (final char[] row : matrix) {
                Arrays.fill(row, ' ');
            }
        }
        
        private void paintButterfly(final int n) {
            paintLeftBorder();
            paintRightBorder();
            
            paintDescendingLine();
            paintAscendingLine();
        }
        
        private void paintLeftBorder() {
            for (final char[] matrix1 : matrix) {
                matrix1[0] = ASTERISK;
            }
        }
        
        private void paintRightBorder() {
            for (final char[] matrix1 : matrix) {
                matrix1[matrix.length - 1] = ASTERISK;
            }
        }
        
        private void paintDescendingLine() {
            for (int i = 0; i < matrix.length; i++) {
                matrix[i][i] = ASTERISK;
            }
        }
        
        private void paintAscendingLine() {
            for (int i = 0; i < matrix.length; i++) {
                matrix[i][matrix.length - i - 1] = ASTERISK;
            }
        }
    }

    public static void main(String[] args) {
        System.out.println(
                ASCIIButterfly
                        .getButterflyArt(
                                parseNfromArguments(args)));
        
        // Benchmark:
        benchmark();
    }
    
    private static int parseNfromArguments(final String[] args) {
        if (args.length == 0) {
            return DEFAULT_N;
        }
        
        final String argument = args[0];
        
        try {
            return Math.max(MINIMUM_N, Integer.parseInt(argument));
        } catch (final NumberFormatException ex) {
            return DEFAULT_N;
        }
    }
    
    private static String convertCharacterMatrixToString(
            final char[][] matrix) {
        
        final int capacity = matrix.length * (matrix.length + 1) - 1;
        final StringBuilder stringBuilder = new StringBuilder(capacity);
        
        for (final char[] row : matrix) {
            stringBuilder.append(row, 0, row.length);
            stringBuilder.append(NEW_LINE);
        }
        
        // Remove the trailing new line:
        stringBuilder.deleteCharAt(stringBuilder.length() - 1); 
        return stringBuilder.toString();
    }
    
    private static void benchmark() {
        long start = System.currentTimeMillis();
        naiveButterfly(50);
        long end = System.currentTimeMillis();
        long naiveDuration = end - start;
        
        start = System.currentTimeMillis();
        String art = ASCIIButterfly.getButterflyArt(50);
        System.out.println(art);
        end = System.currentTimeMillis();
        
        System.out.printf("Naïve Butterfly in: %d milliseconds.\n", 
                          naiveDuration);
        
        System.out.printf("Matrix Butterfly in: %d milliseconds.\n",
                          end - start);
    }
    
    private static void naiveButterfly(final int n) {
        for (int i = 1; i <= n; i++) {
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
            {
                System.out.print("*");
            }
            for(int j=1; j<=2*(n-i); j++){
                System.out.print(" ");
            }
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
            {
                System.out.print("*");
            }
            System.out.println(" ");
        }
        for (int i = n; i >= 1; i--) {
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
                System.out.print("*");
            for(int j=1; j<=2*(n-i); j++){
                System.out.print(" ");
            }
            System.out.print("*");
            for (int j = 1; j <= i - 2; j++){
                System.out.print(" ");
            }
            if(i>1)
                System.out.print("*");
            System.out.println(" ");
        }
    }
}

Typical output

The above benchmark outputs something like:

Naîve Butterfly in: 521 milliseconds.
Matrix Butterfly in: 8 milliseconds.

As you can see, the difference is drastic.

Answer 5

Apart from what is already mentioned, I want to bring out 2 points/good practices:

• Even if you are printing something, it's good to construct the whole string at once and then print it. That way you can use it for other purposes too (saving to file, sending to html page, etc.).
• You can consider linebreak as one of the characters. Then the logic gets (subjectively) easier, not needing to do any kind of "println" logic.
• You can create method to calculate the character at a coordinates. Then the whole code gets (subjectively) a lot easier to read.

    public class SingleLoopButterfly {
    
        public static void main(String[] args) {
            System.out.println(printButterfly(15));
        }
    
        public static char butterflyCharAt(int i, int size) {
            int x = i % (size + 1);
            int y = i / (size + 1);
            if (x == size) {
                return '\n';
            }
            if (x == 0 || x == size - 1 || x == y || y == (size - 1 - x)) {
                return '*';
            }
            return ' ';
        }
    
        public static String printButterfly(int size) {
            StringBuilder sb = new StringBuilder();
            for (int i = 0; i < size * (size + 1); i++) {
    
                sb.append(butterflyCharAt(i, size));
            }
            return sb.toString();
        }
    }

Answer 6

Two thoughts:

1. It is more normal (in C and its descendants, including Java) to write (for your first loop):

   for (int i = 0; i < n; i++) {
   

   I think this is mostly because i then matches up with array indices. It might also optimize better.
2. The line System.out.println(" "); does appear to work, but results in a trailing whitespace on each line. You could also output an empty string, but even that is wasteful. Just write: System.out.println();

You will see both of these in other people's rewrites, but I don't know that anyone has called them out specifically.