3
\$\begingroup\$

To practice my JavaScript for future employment, I've decided to take up the challenge of writing a javascript sudoku verifier. This code only verify's one of the nine 3x3 blocks there are. I would like feedback on this portion of the code, so I can continue my development with more knowledge and more efficient code. Also, any feedback on my HTML/CSS is warmly welcome. I'm using a fullscreen chrome browser for development, so the #middle centers the box on my screen, but it might not on yours.

You will have to click on full page -> to view the CSS correctly.

function checkAnswer() {

	//reset each time button is clicked
	document.getElementById('correct').style.display = 'none';
	document.getElementById('incorrect').style.display = 'none';

	//add each input into 2D array
	let first_row = document.getElementsByClassName('row1');
	let second_row = document.getElementsByClassName('row2');
	let third_row = document.getElementsByClassName('row3');
	let sudoku = [
		[first_row[0].value, first_row[1].value, first_row[2].value],
		[second_row[0].value, second_row[1].value, second_row[2].value],
		[third_row[0].value, third_row[1].value, third_row[2].value]
	]

	//check if each number is unique in the 2D array
	for (let i = 0; i < sudoku.length; i++) {
		for (let j = 0; j < sudoku[i].length; j++) {
			if(!isUnique(sudoku[i][j], sudoku)) {
				document.getElementById('incorrect').style.display = 'block';
				return;
			}
		}
	}
	document.getElementById('correct').style.display = 'block';
}

function isUnique(num, arr) {
	let count = 0;
	//check entire array
	for(let i = 0; i < arr.length; i++) {
		for(let j = 0; j < arr[i].length; j++) {
			if(arr[i][j] == num) {
				count++;
			}
		}
	}
	return count == 1;
}
body { 
	background-color: pink; 
}
input { 
	width: 50px; 
	height: 50px; 
	font-size: 20px; 
	text-align: center; 
}
button { 
	width: 100px; 
	height: 50px; 
	font-size: 15px; 
	margin-left: 40px; 
}

#middle { 
	margin-left: 900px; 
	margin-top: 300px; 
}
<!DOCTYPE html>
<html lang="en-US">
	<head>
		<meta charset='UTF-8'>
		<title>Sudoku</title>
		<link rel="stylesheet" type="text/css" href="style.css">
		<script src="script.js"></script>
	</head>
	<body>
		<div id="middle">
			<table>
				<tr>
					<td><input type="text" class="row1" maxlength="1" value="3"></td>
					<td><input type="text" class="row1" maxlength="1"></td>
					<td><input type="text" class="row1" maxlength="1"></td>
				</tr>
				<tr>
					<td><input type="text" class="row2" maxlength="1"></td>
					<td><input type="text" class="row2" maxlength="1" value="2"></td>
					<td><input type="text" class="row2" maxlength="1"></td>
				</tr>
				<tr>
					<td><input type="text" class="row3" maxlength="1" value="7"></td>
					<td><input type="text" class="row3" maxlength="1"></td>
					<td><input type="text" class="row3" maxlength="1"></td>
				</tr>
			</table>
			<br>
			<button type="button" onclick="checkAnswer()">Check</button>
			<div id="incorrect" style="display: none;">
				<h1>Incorrect Entry!</h1>
			</div>
			<div id="correct" style="display: none;">
				<h1>Correct Entry!</h1>
			</div>
		</div>
	</body>
</html>

Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Styling

You can align centered elements relatively to their parent.

#middle { 
  margin-left: 900px; 
  margin-top: 300px; 
}

#middle {
  position: relative;
  left: 50%;
  transform: translateX(-50%);
  top: 50%;
  transform: translateY(50%);
}

Naming

In my opinion, for algorithms it is OK to use short variable names.

let first_row = document.getElementsByClassName('row1');
let second_row = document.getElementsByClassName('row2');
let third_row = document.getElementsByClassName('row3');
let sudoku = [
  [first_row[0].value, first_row[1].value, first_row[2].value],
  [second_row[0].value, second_row[1].value, second_row[2].value],
  [third_row[0].value, third_row[1].value, third_row[2].value]
]

let r1 = document.getElementsByClassName('row1');
let r2 = document.getElementsByClassName('row2');
let r3 = document.getElementsByClassName('row3');
let sudoku = [
    [r1[0].value, r1[1].value, r1[2].value],
    [r2[0].value, r2[1].value, r2[2].value],
    [r3[0].value, r3[1].value, r3[2].value]
]
Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ Wouldn't it be better to cut out the numbers completely from the variable names? Especially considering a full sudoku is 9x9. \$\endgroup\$
    – Mast ♦
    May 28, 2019 at 6:38
  • 1
    \$\begingroup\$ @Mast In a more generalized sudoku solver, that would be my choice also. \$\endgroup\$
    – dfhwze
    May 28, 2019 at 6:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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