HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Printing a diamond-shaped figure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
diamondprintingfigureshaped

Problem

How can I optimize this code to have fewer loops and return values for unit testing?

public class Diamond {

public void DiamondShape(int num) {

   for(int ucount=num;ucount>0;ucount--) {
//Loop to print blank space
    for(int count_sp=1;count_sp=ucount;count_ast--)
        System.out.printf("* ");
    System.out.println();
   }

//Loop for lower half
  for(int lcount=1;lcount=lcount;count_ast--)
            System.out.printf("* ");
    System.out.println();
    }
  } 
}


Output when num = 3:

*
  * *
 * * *
  * *
   *


This is how the output should be. num indicates the star in center line.

Solution

First of all: Use a StringBuilder instead of System.out.println so you can easily compare the result of your method with an expected output.

Your test could look like: (I renamed you method to draw and moved the size to the instance.)

@Test
public void test4()
{
    StringBuilder expected = new StringBuilder();
    expected.append( "   * \n" );
    expected.append( "  * * \n" );
    expected.append( " * * * \n" );
    expected.append( "* * * * \n" );
    expected.append( " * * * \n" );
    expected.append( "  * * \n" );
    expected.append( "   * \n" );
    assertEquals( expected.toString(), new Diamond( 4 ).draw() );
}


After you have all your tests (I have just created some for 0-4) you can try to extract some methods with duplicate code.

public class Diamond
{
    private int size;

    public Diamond( int size )
    {
        this.size = size;
    }

    public String draw()
    {
        StringBuilder result = new StringBuilder();
        for( int ucount = size; ucount > 0; ucount-- )
        {
            appendSpaces( result, ucount - 1 );
            appendStars( result, size - ucount + 1 );
            newLine( result );
        }

        for( int lcount = 1; lcount < size; lcount++ )
        {
            appendSpaces( result, lcount );
            appendStars( result, size - lcount );
            newLine( result );
        }
        return result.toString();
    }

    private void newLine( StringBuilder result ) // just for better readability
    {
        result.append( "\n" );
    }

    private void appendStars( StringBuilder result, int count ) // just for better readability
    {
        repeat( result, "* ", count );
    }

    private void appendSpaces( StringBuilder result, int count ) // just for better readability
    {
        repeat( result, " ", count );
    }

    private void repeat( StringBuilder result, String string, int count )
    {
        for( int c = 0; c < count; c++ )
            result.append( string );
    }
}


If you want to get rid of one of the loops you could even merge it to:

public String draw()
{
    StringBuilder result = new StringBuilder();
    for( int ucount = size; ucount >= -size; ucount-- )
    {
        boolean isMiddleRows = ucount == 0 || ucount == -1;
        if( isMiddleRows ) continue;
        appendSpaces( result, Math.abs( ucount ) - 1 );
        appendStars( result, size - Math.abs( ucount ) + 1 );
        newLine( result );
    }
    return result.toString();
}


It's pretty cool to have enough tests do try such refactoring :)

Code Snippets

@Test
public void test4()
{
    StringBuilder expected = new StringBuilder();
    expected.append( "   * \n" );
    expected.append( "  * * \n" );
    expected.append( " * * * \n" );
    expected.append( "* * * * \n" );
    expected.append( " * * * \n" );
    expected.append( "  * * \n" );
    expected.append( "   * \n" );
    assertEquals( expected.toString(), new Diamond( 4 ).draw() );
}
public class Diamond
{
    private int size;

    public Diamond( int size )
    {
        this.size = size;
    }

    public String draw()
    {
        StringBuilder result = new StringBuilder();
        for( int ucount = size; ucount > 0; ucount-- )
        {
            appendSpaces( result, ucount - 1 );
            appendStars( result, size - ucount + 1 );
            newLine( result );
        }

        for( int lcount = 1; lcount < size; lcount++ )
        {
            appendSpaces( result, lcount );
            appendStars( result, size - lcount );
            newLine( result );
        }
        return result.toString();
    }

    private void newLine( StringBuilder result ) // just for better readability
    {
        result.append( "\n" );
    }

    private void appendStars( StringBuilder result, int count ) // just for better readability
    {
        repeat( result, "* ", count );
    }

    private void appendSpaces( StringBuilder result, int count ) // just for better readability
    {
        repeat( result, " ", count );
    }

    private void repeat( StringBuilder result, String string, int count )
    {
        for( int c = 0; c < count; c++ )
            result.append( string );
    }
}
public String draw()
{
    StringBuilder result = new StringBuilder();
    for( int ucount = size; ucount >= -size; ucount-- )
    {
        boolean isMiddleRows = ucount == 0 || ucount == -1;
        if( isMiddleRows ) continue;
        appendSpaces( result, Math.abs( ucount ) - 1 );
        appendStars( result, size - Math.abs( ucount ) + 1 );
        newLine( result );
    }
    return result.toString();
}

Context

StackExchange Code Review Q#25099, answer score: 5

Revisions (0)

No revisions yet.