Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

How can i optimize this code with less number of 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_sp++)
        System.out.printf(" ");
//Loop to print *
    for(int count_ast=num;count_ast>=ucount;count_ast--)
        System.out.printf("* ");
    System.out.println();
   }

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

I am new at unit testing want some guidance on unit testing .

Output when num=3

   *
  * *
 * * *
  * *
   *

this is how the output should be, num indicates the star in center line

share|improve this question

2 Answers

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 :)

share|improve this answer
Thanks that is really helpful – Ajit Apr 15 at 9:55
What other tests can we perform in this program – Ajit Apr 17 at 5:22
I think checking the output for 0 to 4 is enough. Maybe you can test negative values, but in general you just need to find all border cases. -1, 0, 1, odd(3), even(4), maybe some larger number too. – mnhg Apr 17 at 5:43
I am raising an exception if value is negative or a string so can we create a test case for the exception that I am raising – Ajit Apr 17 at 11:27
show 3 more comments

If you want to unit test the System.out output, you can set the PrintStream of System.out by using the setOut method.

private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();

@Before
public void setUpStreams() {
    System.setOut(new PrintStream(outContent));
}

@After
public void cleanUpStreams() {
    System.setOut(null);
}

@Test
public void testOut() {
    System.out.println("hello");
    System.out.println("hi");
    String[] linesOfOutput = outContent.toString().split(System.getProperty("line.separator"));
    assertEquals("hello", linesOfOutput[0]);
    assertEquals("hi", linesOfOutput[1]);
}

Of course, it would be better to pass an OutputStream as parameter to the method and write to that.

public void writeDiamondShape ( int num, OutputStream outputStream ) {
    ...
}

As for optimization

public static void writeDiamond(int num){
    int loops = num * 2 - 1;
    int stars = 1;
    for ( int i = 0; i < loops; i++ ) {

        // Print spaces
        int spaces = Math.abs(num - i - 1);
        for ( int j = 0; j < spaces; j++ ) {
            System.out.print ( " " );
        }

        // Print stars
        for ( int j = 0; j < stars; j++ ) {
            System.out.print ( "* " );
        }
        System.out.println();

        // Increment / Decrement stars to print next time
        stars += (i+1 < num) ? 1 : -1;
    }
}
share|improve this answer
1  
I think you should definitely go with the parameter solution if you don't want to use a string return. Changing in, err and out might lead do unexpected behavior, e.g. if you debug a failing test by System.out.print(). – mnhg Apr 15 at 11:07

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.