8
\$\begingroup\$

For this assignment, you will create the pattern of a chess board that is 8 x 8. Use X and O to represent the squares.

  1. Create the appropriate nested looping structure to output the characters in an 8 x 8 grid on the screen using Console.Write() or Console.WriteLine() as appropriate.
  2. Include a decision structure to ensure that alternate rows start with opposite characters as a real chess board alternates the colors among rows.

Simple enough, but my first version stuffed everything inside of Main, which I don't care for. So, I wrote a second version that nicely separated the concerns. The problem with that, in my eyes at least, is that it now does exactly twice the amount of work as my initial "naive" version. Does the benefit of separating concerns outweigh the performance hit on an already exponential algorithm?

Also, I don't like this snippet (that exists in both versions of the code).

if (row % 2 == 0)
{
  grid[row, col] = (col % 2 == 0) ? dark : light;
}
else
{
  grid[row, col] = (col % 2 == 0) ? light : dark; 
}

This differs only in the order of the characters, but I'm too close to it at this point to see a better way to clean up the control logic. Advice on it would be appreciated.

Version 1:

public static void Main()
{
    const int gridSize = 8;
    const string dark = "x";
    const string light = "o";

    for (var row = 0; row < gridSize; row++)
    {
        for (var col = 0; col < gridSize; col++)
        {   
            string output;
            if (row % 2 == 0)
            {
                output = (col % 2 == 0) ? dark : light;
            }
            else
            {
                output = (col % 2 == 0) ? light : dark;
            }

            Console.Write(output);  
        }
        Console.WriteLine();
    }
}

Version 2: (ideone here)

public static void Main()
{
    var grid = CreateGrid(8, 'o', 'x');
    PrintGrid(grid);
}

private static void PrintGrid(char[,] grid)
{
    for (var row = 0; row < grid.GetLength(0); row++)
    {
        for (var col = 0; col < grid.GetLength(0); col++)
        {
            Console.Write(grid[row, col]);
        }
        Console.WriteLine();
    }
}

private static char[,] CreateGrid(int gridSize, char light, char dark)
{
    var grid = new char[gridSize, gridSize];

    for (var row = 0; row < gridSize; row++)
    {
        for (var col = 0; col < gridSize; col++)
        {
            if (row % 2 == 0)
            {
                grid[row, col] = (col % 2 == 0) ? dark : light;
            }
            else
            {
                grid[row, col] = (col % 2 == 0) ? light : dark; 
            }
        }
    }

    return grid;
}
\$\endgroup\$
8
\$\begingroup\$

I like that you have separated the CreateGrid method out in to a reusable method. This is good.

Your logic inside the method is even quite good, despite your concerns about the duplication. There is a trick, though.... use a boolean:

    for (var row = 0; row < gridSize; row++)
    {
        bool toggle = row % 2 == 0;
        for (var col = 0; col < gridSize; col++)
        {
            grid[row, col] = toggle ? dark : light;
            toggle = !toggle;
        }
    }

Your print-grid method would likely benefit from some Linq, but what you have is OK.... rather, it is very readable. Linq (or using a StringBuilder) will improve performance by reducing the calls to Write.

\$\endgroup\$
  • \$\begingroup\$ A StringBuilder is a very good idea indeed. Unfortunately, Linq is a bit out of scope for this lesson. I also very much like the toggle. It's a bit easier to read than the other answer. \$\endgroup\$ – RubberDuck Apr 8 '15 at 13:32
  • \$\begingroup\$ could you declare the toggle outside of the for loops and just start it out as either true or false? whether the board is even by even or odd by odd it shouldn't matter, you wouldn't have to use the row % 2 at all. you wouldn't have to perform a modulus throughout the entire code. \$\endgroup\$ – Malachi Apr 8 '15 at 13:59
  • \$\begingroup\$ @Lyle'sMug - For even-size rows you would not get an alternating pattern - all rows would start with the same colour and you would get stripes, not checkerboard. \$\endgroup\$ – rolfl Apr 8 '15 at 14:00
  • \$\begingroup\$ @Lyle'sMug If we follow your plan, we can do one more toggle outside the inner for loop, to toggle it for the next row. \$\endgroup\$ – Brian J Apr 8 '15 at 21:06
  • 1
    \$\begingroup\$ @Lyle'sMug it wasn't meant to be the same as Rofl's code, so I guess it's confusing :-P I meant something like bool toggle; for { for { toggle } toggle } but I see that only works for even cases, not odd cases. \$\endgroup\$ – Brian J Apr 11 '15 at 2:59
7
\$\begingroup\$

To reduce number of ifs, you could check for a parity of the sum row + col:

for (var row = 0; row < gridSize; row++)
{
    for (var col = 0; col < gridSize; col++)
    {
        grid[row, col] = ((row + col) % 2 == 0) ? dark : light;
    }
}

EDIT
To eliminate the last if in the loop body, you could use the following approach:

string[] colors = { dark, light };
for (var row = 0; row < gridSize; row++)
{
    for (var col = 0; col < gridSize; col++)
    {
        grid[row, col] = colors[(row + col) % 2];
    }
}
\$\endgroup\$
  • \$\begingroup\$ That's pretty genius! \$\endgroup\$ – RubberDuck Apr 8 '15 at 13:24
  • 1
    \$\begingroup\$ @RubberDuck It's math! \$\endgroup\$ – Simon Forsberg Apr 8 '15 at 13:27
1
\$\begingroup\$

You could eliminate the if's completely by using a string array for the alternating colors. This has the added advantage of reducing the inner loop by less than half:

private static string[] CreateGrid(int gridSize, char light, char dark)
{
    var grid = new string[gridSize];
    //Adding the spaces improves the display
    string[] cells = { new string(new char[] { light, ' ', dark, ' ' }), new string(new char[] { dark, ' ', light, ' ' }) };
    int cols = gridSize / 2;
    for (var row = 0; row < gridSize; row++)
    {
        int cellIndex = row % 2;
        StringBuilder sb = new StringBuilder(cells[cellIndex]);
        for (var col = 2; col < cols; col++)
        {
            sb.Append(cells[cellIndex]);
        }
        grid[row] = sb.ToString();
    }
    //Returning a string array reduces the print loop to just 1.
    return grid;
}
\$\endgroup\$
  • \$\begingroup\$ Interesting solution. Sacrifices some readability though. \$\endgroup\$ – RubberDuck Apr 8 '15 at 19:11
1
\$\begingroup\$

In your small example, it does seem like useless to separate the concern. If your program started to be a bit bigger (ex: manipulating the grid) then you would see the usefulness right away.

I don't have a C# compiler to test out right now. But you can create a string from an array of char. This would remove your "col" for loop.

You could use a join to combine each row of strings with a carriage return as the separator.

In the end, I'm pretty sure these still do loop.

\$\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.