7
\$\begingroup\$

I'm working through Eloquent Javascript and just did the examples in chapter 2. One is to create a chess board grid of arbitrary size. My code works, but the if/else if statements I used feel clunky. This is a common feeling I have when I'm writing code, but I'm often at a loss of how else to do it.

How could I write this code in a more compact, elegant way (recognizing the subjectivity of "elegant")?

http://eloquentjavascript.net/02_program_structure.html

var size = 8;
var result = "";
for(var i = 0; i < size; i++) {
  for(var j = 0; j < size; j++) {
    if(i !== 0 && j ===0){
      result += "\n";
    }
    else if((j % 2 === 0 && i % 2 === 0) ||
    		(j % 2 === 1 && i % 2 === 1)){
      result += " ";
    }
    else if((j % 2 === 0 && i % 2 === 1) || 
            (j % 2 === 1 && i % 2 === 0)){
      result += "#"; 
    }
  }
}

console.log(result);

\$\endgroup\$
  • 1
    \$\begingroup\$ The resulting board seems to be flawed; only the first row is 8 characters long while all other are only 7 characters long. \$\endgroup\$ – le_m Jun 7 '17 at 22:19
7
\$\begingroup\$

First of all, your code has a little flaw. It will print the first two lines like this:

◼◻◼◻◼◻◼◻
◼◻◼◻◼◻◼

You may haven't noticed it, because it's hard to spot using a whitespace. This happens, because you use an else if after prepending the newline:

if (i !== 0 && j === 0) {} else if {} else if {}

Remove the first else branch it will work.

Personally, I would move this part to the end of the loop, as it reads more like:

If the line is completed, append a line-break.

This test should be sufficient and is maybe easier to read:

if (size - 1 == j) {
    result += "\n";
}

To take it even a little further, move this out of the inner loop and get rid of the test at all. As you always are going to append a line-break after each row, this is sufficient:

result += "\n";

Thanks to 200_success for pointing that out.


Then you want to print a field of the chessboard in any case. So the second test is actually redundant:

if ((j % 2 === 0 && i % 2 === 0) || (j % 2 === 1 && i % 2 === 1)) {
    result += " ";
} else if((j % 2 === 0 && i % 2 === 1) || (j % 2 === 1 && i % 2 === 0)){
    result += "#"; 
}

… can become:

if ((j % 2 === 0 && i % 2 === 0) || (j % 2 === 1 && i % 2 === 1)) {
    result += " ";
} else {
    result += "#"; 
}

Now, the condition is still hard to read. Let's simplify it:

if (0 === (i + j) % 2) {
    result += "◼";
} else {
    result += "◻";
}

This looks quite simple already. You can alternatively use the ternary operator and rely on JavaScript casting the result to boolean:

result += (i + j) % 2 ? "◻" : "◼";

The final result

var size = 8,
    result = "";

for (var i = 0; i < size; i++) {
    for (var j = 0; j < size; j++) {
        result += (i + j) % 2 ? "◻" : "◼";
    }
    result += "\n";
}

console.log(result);

\$\endgroup\$
  • \$\begingroup\$ if (size - 1 == j) { result += "\n"; } should just be moved out of the inner loop. \$\endgroup\$ – 200_success Jun 7 '17 at 22:18
  • \$\begingroup\$ @200_success Of course, I totally overlooked it. Even the test is unnecessary now. Thanks for your suggestion, I've already updated the answer. \$\endgroup\$ – insertusernamehere Jun 7 '17 at 22:25
3
\$\begingroup\$

My code works, but the if/else if statements I used feel clunky. This is a common feeling I have when I'm writing code.

To improve code readability and reduce the perceived complexity, you need to reduce nesting, remove conditional branches, simplify if-conditions and switch to a more descriptive style. The following 'techniques' help to simplify your code sample:

  1. Exploit the nested loop structure: The first if condition checking for line-breaks becomes true whenever the inner loop terminates. Get rid of the condition and move the statement below the inner loop body:

    var size = 8;
    var result = '';
    for (var i = 0; i < size; i++) {
      for (var j = 0; j < size; j++) {
        if ((j % 2 === 0 && i % 2 === 0) || (j % 2 === 1 && i % 2 === 1)) {
          result += ' ';
        } else if ((j % 2 === 0 && i % 2 === 1) || (j % 2 === 1 && i % 2 === 0)) {
          result += '#'; 
        }
      }
      result += '\n';
    }
    
  2. Remove the redundant else if condition: The final else if condition is redundant as it is always true when the first if-condition is false. Remove it:

    var size = 8;
    var result = '';
    for (var i = 0; i < size; i++) {
      for (var j = 0; j < size; j++) {
        if ((j % 2 === 0 && i % 2 === 0) || (j % 2 === 1 && i % 2 === 1)) {
          result += ' ';
        } else {
          result += '#'; 
        }
      }
      result += '\n';
    }
    
  3. Simplify the if condition: Your if condition can be expressed in terms of both loop iterators and thereby simplified to (j + i) % 2 === 0:

    var size = 8;
    var result = '';
    for (var i = 0; i < size; i++) {
      for (var j = 0; j < size; j++) {
        if ((j + i) % 2 === 0) {
          result += ' ';
        } else {
          result += '#'; 
        }
      }
      result += '\n';
    }
    
  4. Invert the negative if condition: You can get rid of the explicit comparison to zero by switching your if and else statements:

    var size = 8;
    var result = '';
    for (var i = 0; i < size; i++) {
      for (var j = 0; j < size; j++) {
        if ((j + i) % 2) {
          result += '#';
        } else {
          result += ' '; 
        }
      }
      result += '\n';
    }
    
  5. Use the conditional ternary operator for terse conditional assignments: You can replace the somewhat verbose if else statements with a single conditional assignment using the conditional ternary operator:

    var size = 8;
    var result = "";
    for (var i = 0; i < size; i++) {
      for (var j = 0; j < size; j++) {
        result += (j + i) % 2 ? '#' : ' ';
      }
      result += '\n';
    }
    
  6. Replace the declarative loop with a more descriptive approach: Exploit built-in methods and introduce new named identifiers for self-documenting code:

    function createBoard(size, black = '#', white = ' ') {
      let even = (white + black).repeat(size / 2) + (size % 2 ? white : '') + '\n';
      let odd  = (black + white).repeat(size / 2) + (size % 2 ? black : '') + '\n';
      return     (even  + odd  ).repeat(size / 2) + (size % 2 ? even  : '');
    }
    
    console.log(createBoard(8));
    

Especially the last two changes are subject to personal preferences - some prefer the declarative style, some stick to if { ... } else { ... } everywhere - it's your (team's) choice.

\$\endgroup\$
2
\$\begingroup\$

I would consider building the board as a separate concern from displaying the board. I say this as it is more practical from a learning standpoint to understand how you might build the data structure that represents a chessboard (for example a two-dimensional array) not, just how to display it by building a string. During your programming journey, you will likely learn to embrace the separation of logic/data storage concerns from display concerns.

I would also encourage you to more heavily leverage Array() functions.

This might yield something like:

function buildBoard(size) {
    var board = Array(size);
    for (var row = 0; row < size; row++) {
        board[row] = Array(size).fill(0);
        for (var col = row % 2; col < size; col = col + 2) {
            board[row][col] = 1;
        }
    }
    return board;
}

function displayBoard(size) {
    var board = buildBoard(size);
    var output = '';
    board.forEach(function(row) {
        row.forEach(function(col) {
             output += (col === 1) ? '#' : ' ';
        });
        output += "\n";
    });
    console.log(output);
}

displayBoard(8);
displayBoard(20);

Now this certainly is not optimized from the standpoint of operational complexity (in that you have to perform \$O(n^2)\$ iteration twice, but it does break down the problem into the logic space of setting up a board and then displaying a board, which has more real-world applicability than building a chessboard directly into a string.

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