11
\$\begingroup\$

I have an assignment to code a method that will print values in a diamond pattern. For example, if the method is called like printNumberDiamond(2), then the results should look like:

"  0  "
" 010 "
"01210"
" 010 "
"  0  "

I have spent a ton of time on writing the code that does this, but I am sure that it's not the most efficient/clean way. I am hoping to learn new strategies for accomplishing this, because this was very difficult for me to complete, and I almost just gave up, but I got it to work:

public static void printNumberDiamond(int diaCenter)
{   
        printNumberDiamondTop(diaCenter);
        printNumberDiamondBottom(diaCenter);
}


public static void printNumberDiamondTop(int diaCenter)
{
    int maxAscendingRows = diaCenter + 1;                       
    int maxColumnLength = diaCenter * 2 + 1;            
    int highestNbrForRow = 0;

    for(int row = 0, rowCtr=0;row<=diaCenter;row++)
    {   
        int thisColumnLength = 0;
        int thisColumnMaxLeftSpaces = 0;        

        if(row >= maxAscendingRows)
        {           
            thisColumnMaxLeftSpaces = (maxColumnLength - rowCtr) / 2;
            rowCtr--;           
        }
        else
        {
            thisColumnLength = maxColumnLength - row;   
            thisColumnMaxLeftSpaces = (thisColumnLength - row) / 2;
            rowCtr++;
        }

        int nbrWritten = 0;
        int nbrDescendingWritten = 0;           
        for(int col = 0;col < maxColumnLength;col++)            
        {
            if(col < thisColumnMaxLeftSpaces)           
            {
                System.out.print(" ");          
            }
            else if(nbrDescendingWritten == 0 && nbrWritten > row)  
            {
                System.out.print(" ");          
            }
            else if(nbrWritten <= highestNbrForRow)     
            {
                System.out.print(nbrWritten++);     
                nbrDescendingWritten = nbrWritten - 1;                  
            }           
            else if(nbrWritten > highestNbrForRow)
            {
                System.out.print(--nbrDescendingWritten);                   
            }                           
        }           
        ++highestNbrForRow;
        System.out.print("\n");
    }
}

public static void printNumberDiamondBottom(int diaCenter)
{               
    int maxColumnLength = diaCenter * 2 + 1;                        
    int highestNbrForRow = 0;                                       
    for(int row = diaCenter;row>0;row--)
    {               
        highestNbrForRow = row - 1;
        int thisColumnMaxLeftSpaces = (maxColumnLength - highestNbrForRow * 2) / 2;
        int nbrWritten = 0;
        int nbrDescendingWritten = 0;

        for(int col = 0;col < maxColumnLength;col++)            
        {
            if(col < thisColumnMaxLeftSpaces)           
            {
                System.out.print(" ");          
            }
            else if(nbrDescendingWritten == 0 && nbrWritten > highestNbrForRow) 
            {
                System.out.print(" ");          
            }
            else if(nbrWritten <= highestNbrForRow && nbrWritten >= 0)      
            {
                System.out.print(nbrWritten++);     
                nbrDescendingWritten = nbrWritten - 1;                  
            }           
            else if(nbrWritten > highestNbrForRow)
            {
                System.out.print(--nbrDescendingWritten);                   
            }                           
        }           
        ++highestNbrForRow;
        System.out.print("\n");
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Great question! While I'm refactoring it, I will give you a general hint: look at your two methods: printNumberDiamondTop and printNumberDiamondBottom and spot places where you have common code, then extract this common code fragments to a separate methods. This will shorten your code and make it more clear. \$\endgroup\$ – RK1 Jul 7 '16 at 16:37
  • 7
    \$\begingroup\$ I've edited out the part where you ask reviewers to post alternative solutions; while reviewers may present better/alternative solutions, they don't have to. You will find that answerers on this site address any & all aspects of the code; sometimes the best answers don't even include a code block. Feel free to hop into The 2nd Monitor if you have any questions. \$\endgroup\$ – Mathieu Guindon Jul 7 '16 at 16:41
  • \$\begingroup\$ Hope you don't mind I turned this into a golfing challenge :) \$\endgroup\$ – Insane Jul 8 '16 at 4:40
12
\$\begingroup\$

Before going into any coding I would give you a quick hack for the diamond problem . Instead of making so many for looping and columns and keeping track of variables the diamond problem is a variant of a maths problem. HERE

1*1 = 1
11*11 = 121
111*111 = 12321
1111*1111 = 1234321 

and so on. SO if you use this formula you just need one for loop for spaces and one for increasing your num*10 +1 where num is 1 and then squaring it and you will get the diamond pattern. If you want I can code it for you but since you asked for an easier method this is the best you can get.

\$\endgroup\$
  • 2
    \$\begingroup\$ This method will work only till printNumberDiamond(10) after that output will be different. \$\endgroup\$ – Kaushal28 Jul 7 '16 at 17:02
  • 3
    \$\begingroup\$ For n>9 it is not clear what the output pattern should look like \$\endgroup\$ – FredK Jul 7 '16 at 17:04
  • \$\begingroup\$ Agreed that this works for only n <= 9 \$\endgroup\$ – Abhinav Pandey Jul 7 '16 at 18:13
  • 2
    \$\begingroup\$ Thank you, this is very useful. No wonder the assignment said that the parameter value should always be <= 9. \$\endgroup\$ – Chris Phillips Jul 7 '16 at 20:10
6
\$\begingroup\$

This method is immediately smelly:

public static void printNumberDiamond(int diaCenter)
{   
        printNumberDiamondTop(diaCenter);
        printNumberDiamondBottom(diaCenter);
}

Which of the called methods will print the center line? It's impossible to tell without looking at the implementation, and you don't want readers to have to ask such questions. You want to make the logic as obvious as possible.

A simple step that will solve this issue is to add a third method to print the center line, and call it between these two methods.

Then, as a next step, realize that since the bottom is a mirror of the top, the logic to print them should have some common element. Looking at this code, or looks like there is no common element, and it will be a good idea to reorganize the code to make that happen.

\$\endgroup\$
  • \$\begingroup\$ Thanks for pointing that out! I definitely agree that the less you have to really dig into the code to understand what it's doing, the better it is. I will remember your suggestion going forward. \$\endgroup\$ – Chris Phillips Jul 7 '16 at 20:18
5
\$\begingroup\$

Determine the number of lines to output (2*n + 1). then for each line:

  • determine how many leading spaces (m) and the highest digit t
  • print m spaces
  • print the integers from 0 through t
  • print the digits from t-1 down to zero
  • print a newline

Here's code that works for n=0..15

public static void printTriangle(int n ) {
  int lines = (2 * n) + 1;
  for (int i = 0; i < lines; i++) {
     int nsp;
     int top;
     if ( i <= n ) {
        nsp = n - i;
        top = i;
     } else {
        nsp = i - n;
        top = n - nsp;
     }
     // print out nsp spaces
     for (int j = 0; j < nsp; j++) {
        System.out.print( ' ' );
     }

     // print integers 0 through top
     for (int j = 0; j <= top; j++) {
        System.out.printf( "%x", j );
     }
     // ptint numbers from top-1 to zero
     for (int j = top - 1; j >= 0; j--) {
        System.out.printf( "%x", j );
     }
     System.out.println( "" );
  }
}
\$\endgroup\$
  • \$\begingroup\$ I can't get this one to print with the spaces to the right. For example, I'm getting " 0" on the first line rather than " 0 " where there are 2 spaces before the zero and 2 spaces after the zero. \$\endgroup\$ – Chris Phillips Jul 7 '16 at 20:06
5
\$\begingroup\$

I would approach this a two step problem. Build the pattern, then print the pattern. When you try to build it as you print it, you are forced to think left-to-right and then top-to-bottom. Instead of being able to formulate an appropriate mathematical model.

Abhinav Pandey's answer is a good solution for the n <= 9 limit, but there are other models beyond that. You know that the resultant matrix will be 2n+1 by 2n+1, so allocate an array of that size and then fill it in by starting in the center and working your way out.

Printing it out is now a very direct nested loop.

\$\endgroup\$
  • \$\begingroup\$ Thank you, that's a really good way of thinking about the solution, and I agree with you! \$\endgroup\$ – Chris Phillips Jul 7 '16 at 20:16
5
\$\begingroup\$

Solution suggested by FredK is something I would implement, as it breaks down the problem to more smaller problems. However, this only prints the upper part of the diamond, so I would suggest doing the following:

  • don't immidiately print characters out, store them in a String and when a line is ready, store it in an array, or a List, called, for example, lines

  • when you are done with upper half of the diamond, print all the lines, each one in a new line

  • reverse the lines and remove the first element (to avoid duplicating the middle line)

  • print the reversed lines for the last time


EDIT: Since you said you already implemented the program, here is my solution for comparison - notice that it will also work for numbers greater than 9, but you can change the implementation of the getMiddleDigits method and still have the correct diamond printed out.

import java.util.*;

public class DiamondPrinter {

    public static void main(String[] args) {
        DiamondPrinter printer = new DiamondPrinter();
        printer.printNumberDiamond(5);
    }

    public void printNumberDiamond(int depth) {
        List<String> lines = new ArrayList<>();
        int emptySpacesInCurrentLine = depth;
        for (int lineNumber = 0; lineNumber <= depth; lineNumber++) {
            String line = "";
            String spaces = getEmptySpaces(emptySpacesInCurrentLine);
            String middleDigits = getMiddleDigits(lineNumber);
            lines.add(line.concat(spaces).concat(middleDigits).concat(spaces));
            emptySpacesInCurrentLine--;
        }
        printLines(lines);
        lines = getBottomPart(lines);
        printLines(lines);
    }

    private String getEmptySpaces(int emptySpaces) {
        String result = "";
        for (int i = 0; i <= emptySpaces; i++) {
            result = result.concat(" ");
        }
        return result;

    }

    private String getMiddleDigits(int number) {
        String result = "";
        for (int i = 0; i <= number; i++) {
            result = result.concat(String.valueOf(i));
        }
        for (int i = number - 1; i >= 0; i--) {
            result = result.concat(String.valueOf(i));
        }

        return result;
    }

    private List<String> getBottomPart(List<String> lines) {
        Collections.reverse(lines);
        lines.remove(0);
        return lines;
    }

    private void printLines(List<String> lines) {
        for (String line : lines) {
            System.out.println(line);
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ That's a great idea, and I've already implemented your suggestion! \$\endgroup\$ – Chris Phillips Jul 7 '16 at 20:21
  • \$\begingroup\$ Great. Since you already implemented it, I edited the answer with my solution, just for comparison. \$\endgroup\$ – RK1 Jul 7 '16 at 20:45
3
\$\begingroup\$

I'd do it as follows:

String diamond(int n) {
    StringBuilder sb = new StringBuilder();
    for (int y = -n; y <= n; y++) {
        for (int x = -n; x <= n; x++) {
            int value = n - Math.abs(x) - Math.abs(y);
            sb.append(value >= 0 ? value : " ");
        }
        sb.append("\n");
    }
    return sb.toString();
}

because

  • the code is reasonably short
  • the nested loops make it obvious that we are iterating over a matrix, and adding a character for every cell
  • returning a String rather than printing it directly makes unit testing easier (not strictly needed here, but generally a good practice to follow), may be more efficient (because there is only a single write to the underlying stream rather than a write for each line or character), and more flexible if you want to print your diamond elsewhere instead (separation of concerns: generating vs. printing the diamond)
\$\endgroup\$
  • \$\begingroup\$ This solution is near to irreducible complexity. \$\endgroup\$ – oopexpert Jul 7 '16 at 22:21
  • \$\begingroup\$ @oopexpert Meaning it's nearly as clean as it can possibly get? \$\endgroup\$ – David Moles Jul 8 '16 at 18:48
  • \$\begingroup\$ No, but it seems to be a local optimum on the properties of "functionality" and "shortness". I did not mention other non-functional requirements. As this solution is so short someone has a hard time to argue a violation of any of them. So that's why I did not adress them and my comment is hort as well. \$\endgroup\$ – oopexpert Jul 8 '16 at 20:35
2
\$\begingroup\$

The diamond is simmetric, so in pseudocode:

Print_diamond = do {print top; print center; print reverse(top)}

The center is easy to generate:

0.upto(n) + (n - 1).downto(0)

Then reduce it step by step by replacing the center 3 items with the center item minus one until you arrive to a single 0:

Example: 01210 
          010
121 becomes 010
010 becomes 0
Stop

The number of spaces is equal to the max line length - current line length, half before, half after the digits.

Separate this steps into functions and you will have clear code.


When coding create a plan, divide it into steps and then write the actual code. Writing many nested for loops and conditions without an overall view rarely works out for more complex problems (even if it does it is fragile to the addition of functionality and changes).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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