以前に投稿して随分と時間が経ってますが、たまにストックしてくださったり見てくださる方がいらっしゃいます。
より良く新しい記述を提供できたらと思ったので今回リファクタリングを試みました。
初学者の方のお役に立てればと思います。
以前投稿した記事はこちら
以前のコード
window.onload = function(){
var $tableElements = document.getElementsByTagName('td');
//順番を制御するための変数
let order = true; //trueは黒(先行)
let othelloWhte = '◯';
let othelloBlack = '●';
let othelloColor = othelloBlack;
//tableの全てにclickイベントを付与する
for (let $i=0; $i < $tableElements.length; $i++) {
$tableElements[$i].addEventListener('click', function(){
//配列に変換する
let tableElements = [].slice.call($tableElements);
//クリックした位置の取得
let index = tableElements.indexOf(this);
putOthello(index);
changeOrder();
});
}
function putOthello(index) {
$tableElements[index].innerHTML = othelloColor;
}
//順番の判別する
function changeOrder() {
if (order) {
othelloColor = othelloWhte;
order = false;
} else {
othelloColor = othelloBlack;
order = true;
}
}
}
修正点
- othelloWhte →スペルミス
- var $tableElements → なぜかvarで定義されている
- 順番を制御するための変数やオセロの色の箇所もっとすっきりできるはず
- 配列の変換→スプレット演算子活用
- for文の変数がなぜか$i (PHPでも書いていたつもりだったのか?)
- 順番を判別する関数はまるっと必要ないと思ったので消す
いろいろと恥ずかしい感じになっているのを直しました…
window.onload = function() {
// tdを取得
const $tableElements = document.getElementsByTagName('td');
// 先手 true: 黒、false:白
let order = true;
// tableの全てにclickイベントを付与する
for (let i = 0; i < $tableElements.length; i++) {
$tableElements[i].addEventListener('click', function() {
// 配列に変換する
const tableElements = [...$tableElements];
// クリックした位置の取得
const tableIndex = tableElements.indexOf(this);
putOthello(tableIndex);
});
}
/**
* @param {number} index
*/
function putOthello(index) {
// すでにオセロが置かれていた場合は処理をさせない
if ($tableElements[index].innerHTML) return false;
// ifを省略してtrueなら右側、falseなら左側が代入されるように記述
const othello = (order) ? '●' : '◯';
$tableElements[index].innerHTML = othello;
// 最後に順番を反転させる
order = !order;
}
}
わりとすっきりした気がします。
ついでにすでにオセロが置いてある箇所は無駄な処理が走らないように早期リターンさせておく記述を追記しておきました。
引数に何が入ってくるかをコメントに書いてわかりやすくしておきました。
この時点ではオセロをただおくだけなので、次回は以前書いたカオスな置き換え処理をもう少し綺麗に書けないか考えてみます。