Check if my matrix is a magic square javascript

Question

We have been given an assignment to create a 3x3 matrix using input boxes and make a code to check if the given numbers add up correctly to make is a magic square. No prompt boxes and for us to use an array to accomplish the task. I have used input boxes to get values before using the "document.myform....value" method, but I have no idea how to implement that with an array.

This is what I have so far:

function myArray() {
  var matrix = new Array[];
  for (var i = 0; i < 3; i++) {
    matrix[i] = new Array(3)
  }
  enterValues();
}

function enterNum() {
  for (var row = 0; row < 3; row++) {
    for (var col = 0; col < 3; col++)
      matrix[row][col].innerHTML;
  }
}

function magicSquare() {
  var magSqu = false;
  var sum = 0;
  for (var col = 0; col < 3; col++) {
    sum = sum + matrix[0][col];
  }
  var rowSum = 0;
  for (var i = 1; i < 3; i++) {
    for (var j = 0; j < 3; j++) {
      rowSum = rowSum + matrix[i][j];
      if (rowSum != sum) {
        magSqu = false;
        break;
      }
    }
  }
  var sum_diagonal = 0;
  if (magSqu) {
    for (var i = 0; i < 3; i++)
      for (var j = 0; j < 3; j++)
        if (i == j)
          sum_diagonal += matrix[i][j];
    if (sum_diagonal != sum) {
      magSqu = false;
    }
  }
  if (magSqu) {
    sum_diagonal = 0;
    for (var i = 0; i < 3; i++)
      for (var j = 0; j < 3; j++)
        if ((i + j) == 2)
          sum_diagonal += matrix[i][j];
    if (sum_diagonal != sum) {
      magSqu = false;
    }
  }
  if (magSqu == true) {
    document.getElementById("result").innerHTML = "This is a magic square.";
  } else {
    document.getElementById("result").innerHTML = "This is not a magic square.";
  }
}

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="utf-8">
  <title>Magic Square</title>
  <link rel="stylesheet" type="text/css" href="style.css" />

</head>

<body>
  <table border='1'>
    <tbody>
      <tr>
        <td id="r1c1"><input type="text" name="inp11"></td>
        <td id="r1c2"><input type="text" name="inp12"></td>
        <td id="r1c3"><input type="text" name="inp13"></td>
      </tr>
      <tr>
        <td id="r2c1"><input type="text" name="inp21"></td>
        <td id="r2c2"><input type="text" name="inp22"></td>
        <td id="r2c3"><input type="text" name="inp23"></td>
      </tr>
      <tr>
        <td id="r3c1"><input type="text" name="inp31"></td>
        <td id="r3c2"><input type="text" name="inp32"></td>
        <td id="r3c3"><input type="text" name="inp33"></td>
      </tr>
    </tbody>
  </table>
  <input type="submit" id="magicButton" value="Compute Score" onclick="magicSquare();">
  <div id="result"> </div>
</body>

</html>

Where am I going wrong with this?

There's a typo you might want to fix: `new Array[]`. Should be `[]` or `Array()`. — ggorlen, Nov 24 '19 at 02:35

score 1 · Accepted Answer · answered Nov 24 '19 at 03:11

Most of the things you missed, were

Defining array as [] or new Array(), which was said by @ggorlen
Instead of using name I've changed it to id, so that we can get that particular element's value.
when working with input tags, get the value inside using the value property and not innerHTML
We need to call the function on initial load myArray()
Move the matrix variable to the top level, since its used commonly by all functions.
Removed the input button of type submit, this is optional, since it helped retain the inputs for demo purposes.

Hope this solves your issue!

var matrix = [];
myArray();
function myArray() {
  for (var i = 0; i < 3; i++) {
    matrix[i] = new Array(3).fill(1)
  }
  enterValues();
}

function enterValues() {
  for (var row = 0; row < 3; row++) {
    for (var col = 0; col < 3; col++) {
      document.getElementById(`inp` + (row+ 1) + (col+ 1)).value = matrix[row][col];
    }
  }
}

function enterNum() {
  for (var row = 0; row < 3; row++) {
    for (var col = 0; col < 3; col++)
      matrix[row][col] = document.getElementById(`inp` + (row+ 1) + (col+ 1)).value;
  }
}

function magicSquare() {
  enterNum();
  var magSqu = false;
  var sum = 0;
  for (var col = 0; col < 3; col++) {
    sum = sum + matrix[0][col];
  }
  var rowSum = 0;
  for (var i = 1; i < 3; i++) {
    for (var j = 0; j < 3; j++) {
      rowSum = rowSum + matrix[i][j];
      if (rowSum != sum) {
        magSqu = false;
        break;
      }
    }
  }
  var sum_diagonal = 0;
  if (magSqu) {
    for (var i = 0; i < 3; i++)
      for (var j = 0; j < 3; j++)
        if (i == j)
          sum_diagonal += matrix[i][j];
    if (sum_diagonal != sum) {
      magSqu = false;
    }
  }
  if (magSqu) {
    sum_diagonal = 0;
    for (var i = 0; i < 3; i++)
      for (var j = 0; j < 3; j++)
        if ((i + j) == 2)
          sum_diagonal += matrix[i][j];
    if (sum_diagonal != sum) {
      magSqu = false;
    }
  }
  if (magSqu == true) {
    document.getElementById("result").innerHTML = "This is a magic square.";
  } else {
    document.getElementById("result").innerHTML = "This is not a magic square.";
  }
}

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="utf-8">
  <title>Magic Square</title>
  <link rel="stylesheet" type="text/css" href="style.css" />

</head>

<body>
  <table border='1'>
    <tbody>
      <tr>
        <td id="r1c1"><input type="text" id="inp11"></td>
        <td id="r1c2"><input type="text" id="inp12"></td>
        <td id="r1c3"><input type="text" id="inp13"></td>
      </tr>
      <tr>
        <td id="r2c1"><input type="text" id="inp21"></td>
        <td id="r2c2"><input type="text" id="inp22"></td>
        <td id="r2c3"><input type="text" id="inp23"></td>
      </tr>
      <tr>
        <td id="r3c1"><input type="text" id="inp31"></td>
        <td id="r3c2"><input type="text" id="inp32"></td>
        <td id="r3c3"><input type="text" id="inp33"></td>
      </tr>
    </tbody>
  </table>
  <input type="button" id="magicButton" value="Compute Score" onclick="magicSquare();">
  <div id="result"> </div>
</body>

</html>

Although, I have played around with it, and no matter what numbers I put in it always equals out to "Not a square"--even the number sequences I know equal out to 15. — Ellis Jordan, Nov 24 '19 at 23:46
@EllisJordan I explain all of the bugs in my answer. `if (rowSum != sum)` in your loops is premature and fails every time. There's no check that all numbers are unique. This answer does not address these issues, only seems to attempt to get you back on track in terms of fixing your syntax error and hooking you up to the DOM, which is a good step. — ggorlen, Nov 25 '19 at 17:22

ggorlen · Answer 2 · 2020-01-06T05:45:34.417

There are many issues in the code. My general advice is to work in very small chunks and solve one problem at a time. The code looks as if it was written in one fell swoop, without being tested at regular intervals to ensure that it works. By the time you get around to running it, the number of issues has accumulated to a point where it's difficult to debug.

Some of the issues include:

A lack of interaction with the DOM. Setting the result using innerHTML is a good start, but there's no logic to bring the data in from the user. Use a form, event listener or query selector to extract data from the document. Ensure this works before moving on to the magic number logic (or hardcode data and figure out the magic number logic before moving on to the UI). The UI and magic number logic should be totally decoupled from one another.
Scoping: myArray() is the function that you've set up to store the matrix (breaking the code into functions was the right idea!), but it's never called and doesn't return the matrix for the other functions to use. Although you've done a nice job of breaking the code into functions, be sure to use parameters and return values and further break row/column/diagonal checks into their own functions. Give functions and variables descriptive names (you've generally done a good job of this, myArray notwithstanding).
matrix[row][col].innerHTML; doesn't do anything (use an = sign to assign to innerHTML or use the property in an expression), nor is enterValues() called. This appears to be a work in progress, but this could be a good function to isolate and get working to pull the data from the DOM.

Use braces on all blocks:

for (var i = 0; i < 3; i++)
  for (var j = 0; j < 3; j++)
    if (i == j)
      sum_diagonal += matrix[i][j];
if (sum_diagonal != sum) {
  magSqu = false;
}

is a disaster waiting to happen, although it does appear correct here. sum_diagonal should be sumDiagonal (or diagonalSum)--JS uses camelCase for all variables (PascalCase for classes). The inner loop isn't necessary. A rewrite might be:

for (let i = 0; i < 3; i++) {
  sumDiagonal += matrix[i][i];
}

if (sumDiagonal !== sum) {
  magSqu = false;
}

Hardcoding 3 works here, but I'd try to generalize the logic to work on any n.
Use === for all comparisons. This avoids nasty bugs due to unanticipated type coercion.
Your magic square tests fail to account for situations where the numbers aren't distinct. This would incorrectly consider a 3x3 matrix of all zeroes to be a magic square. I'd recommend flattening the list and using a Set to check that every element is unique.

Another bug is the following code:

var rowSum = 0;
for (var i = 1; i < 3; i++) {
  for (var j = 0; j < 3; j++) {
    rowSum = rowSum + matrix[i][j];
    if (rowSum != sum) { // <-- this is premature!
      magSqu = false;
      break;
    }
  }
}

In the above snippet, the row sum should only be tested after the row is finished summing, not during the summation. It will incorrectly report false. Try:

let rowSum = 0;

for (let i = 1; i < 3; i++) {
  for (let j = 0; j < 3; j++) {
    rowSum += matrix[i][j];            
  }

  if (rowSum !== sum) {
    magSqu = false;
    break;
  }
}

With this in mind, I prefer a solution that breaks the task into small functions that use array builtins to perform iteration instead of explicit loops:

const sum = a => a.reduce((a, e) => a + e);
const diag = m => m.map((row, i) => row[i]);
const rotate = m => 
  m.map((_, i) => m.map((_, j) => m[j][i]).reverse())
;
const isMagic = square => {
  const targetSum = square.length && sum(square[0]);
  const valid = a => sum(a) === targetSum;
  const rotated = rotate(square);  
  const flat = square.flat();
  return new Set(flat).size === flat.length &&
    square.every(valid) &&
    rotated.every(valid) &&
    valid(diag(square)) &&
    valid(diag(rotated))
};

[
  [
    [2,7,6],
    [9,5,1],
    [4,3,8]
  ],
  [
    [8,1,6],
    [3,5,7],
    [4,9,2]
  ],
].forEach(e => console.log(isMagic(e)));

This fulfills all of the requirements for a magic square: all numbers should be unique and every row, column and diagonal must sum to the same number.

With a permutation function, I can test this to make sure it finds all of the expected squares of a given size. Caution: this is a heavy computation that can hang the browser for a few moments.

const sum = a => a.reduce((a, e) => a + e);
const diag = m => m.map((row, i) => row[i]);
const rotate = m => 
  m.map((e, i) => e.map((_, j) => m[j][i]).reverse())
;
const isMagic = square => {
  const targetSum = square.length && sum(square[0]);
  const valid = a => sum(a) === targetSum;
  const rotated = rotate(square);  
  const flat = square.flat();
  return new Set(flat).size === flat.length &&
    square.every(valid) &&
    rotated.every(valid) &&
    valid(diag(square)) &&
    valid(diag(rotated))
};

function *permute(a, i=0) {
  if (i >= a.length) {
    yield a.slice();
  }

  for (let j = i; j < a.length; j++) {
    [a[i], a[j]] = [a[j], a[i]];
    yield *permute(a, i + 1);
    [a[i], a[j]] = [a[j], a[i]];
  }
};

const unflatten = a => a.reduce((acc, e, i) => {
  if (i % Math.sqrt(a.length) === 0) {
    acc.push([]);
  }
  
  acc[acc.length-1].push(e);
  return acc;
}, []);

const squares = [...permute(
    Array(9).fill().map((_, i) => i + 1)
  )].reduce((a, e) => {
    const candidate = unflatten(e);

    if (isMagic(candidate)) {
      a.push(candidate);
    }

    return a;
  }, [])
;

console.log(squares.length, "magic squares of size 3:");

for (const square of squares) {
  console.log(square.map(e => e.join(" ")).join("\n"));
}

Check if my matrix is a magic square javascript

2 Answers2