Printing a Φ pattern using * characters

Question

So I am a newbie obviously, This was part of a small assignment I had to do for Uni, and it works alright. It just reads a number from the console and then outputs the Greek letter Φ (sort of). Sure it could use some outofbound exceptions etc, but my question is this: My code overall (not just in this example) seems a little bit messy to me after I read it. My variable names are messy and I need to work on that and perhaps a few methods here and there, even though this assignment wasn't about methods but rather a warm up of basic Java syntax.

I want to write better code, but I don't know where to start.Should I use more variables? I am not greatly familiar with JavaDocs yet, should they be used for small programs as well? How much procedures is too much? Should I break it down more? I feel like my "algorithm" even though it works it could be better. Online tutorials usually teach syntax but I want to make my code as good and readable as possible.

import java.util.*;

public class A11{

    public static void main(String args[]){

        /*Declare Num, then parse the number from the command line 
        into the variable,and finaly inform the user before moving on*/
        int temp=0;
        int num =0;
        num=Integer.parseInt(args[0]);
        System.out.println();
        System.out.println("Number = " + num);
        temp=num/2;

        /*Prints the first Line (approp spaces and a Star)*/
        if(num%2==0){
            temp-=1;
        }
        for(int v=0;v<=temp;v++){

            System.out.print(" ");
        }
        System.out.println("*");


        /*Print the remaining Shape according to the input*/

        /* Loop algorithm to calculate and output the appropreate amount of
        characters required to form the TOP half of the shape */
        if(num%2==0){temp=num/2-1;}

        for(int i=1;i<=temp;i++){   

            for(int j=0;j<temp-i;j++){              
                System.out.print(" ");                              
            }           
            System.out.print("*");                  
            for(int k=0;k<2;k++){
                for(int j=0;j<i;j++){               
                    System.out.print(" ");
                }
                System.out.print("*");
            }                       
            System.out.println();                       
        }
    /*Bottom half - if Num is an odd number flip the top shape but loose one line*/
        int space=temp;
         if(num%2==1){
            temp-=1; 
         }  
        for(int i=temp;i>=1;i--){       

            for(int j=space-i;j>0;j--){         
                System.out.print(" ");                              
            }           
            System.out.print("*");                  
            for(int k=0;k<2;k++){
                for(int j=0;j<i;j++){               
                    System.out.print(" ");
                }
                System.out.print("*");      
}               
            System.out.println();                       
        }
        temp=num/2;
        /*Prints the last line and Star*/
        if(num%2==0){
            temp-=1;
        }
        for(int v=0;v<=temp;v++){

            System.out.print(" ");
        }
        System.out.println("*");                
    }       
}

Answer 1

Your use of the temp variable makes your code particularly infuriating to follow:

• It's not even that temporary in scope: it's used throughout the function!
• temp is almost never a good name for a variable, since it doesn't give any clue what it is supposed to represent.
• You redefine and repurpose it several times within the function.
• There are too many if(num%2==0) special cases. You shouldn't need any special cases if you just use the truncation that naturally happens with integer division in your favour.

For that matter, num is also a meaningless variable name. I can see that it's an int, which is, obviously, a number. How about size instead?

There's a lot of copy-and-paste code. You could benefit greatly splitting the work into functions. Decomposing the task would also force you to think about what the subroutines are, and help you organize your code.

Calling System.out.print() to print each character at a time is very tedious. It's also bad for performance, if that matters to you. A much better strategy would be to create a buffer to print at least a line at a time, if not the entire picture at once. Another benefit of using a buffer is that you can just place the stars in the right place using a formula.

Suggested solution

import java.util.Arrays;

public class Phi {
    /**
     * Produces a string of spaces that has the character <code>c</code>
     * at the three specified indices, followed by a newline.
     */
    private static String row(char c, int left, int mid, int right) {
        char[] line = new char[right + 2];
        Arrays.fill(line, ' ');
        line[left] = line[mid] = line[right] = c;
        line[right + 1] = '\n';
        return new String(line);
    }

    public static String toString(char c, int size) {
        StringBuilder out = new StringBuilder();
        int halfWidth = (size + 1) / 2;

        // Top stem
        out.append(row(c, halfWidth, halfWidth, halfWidth));

        // Body
        for (int row = 2; row <= size / 2; row++) {
            out.append(row(c, halfWidth - row, halfWidth, halfWidth + row));
        }
        for (int row = halfWidth; row >= 2; row--) {
            out.append(row(c, halfWidth - row, halfWidth, halfWidth + row));
        }

        // Bottom stem
        out.append(row(c, halfWidth, halfWidth, halfWidth));

        return out.toString();
    }

    public static void main(String[] args) {
        int size = Integer.parseInt(args[0]);
        System.out.printf("\nNumber = %d\n%s", size, toString('*', size));
    }
}