投稿者: 
現在、もりけん塾にてJavaScriptを勉強させていただいております。その記録。
今回は課題22についての備忘録です。
考え方が間違っている箇所もあるかと思いますが、お気づきの際はご指摘いただければと思います。
課題22について
仕様
①課題21の仕様
- こちらのようなテーブルを画面遷移してから3秒後に解決されるPromiseが返すオブジェクトを元に作り、 idがソートできる機能を作ってください
- ソートは通常時はidが適当でもよく
- ソートが昇順の場合は上矢印がアクティブ、下矢印がdisabled、1,2,3,4,5の順番で表示され、降順の場合はその逆、
通常時の矢印クリック(クリッカブル領域は2つの矢印です。上下別々のクリッカブル領域でではなく)を押すと画像のように変化します
②課題22の仕様
同じことを年齢でもやってください。
制作物
コードはこちらから(codesandbox)※codesandboxではエラーが出る場合がありますが、リロードすれば動きます。
実装内容
課題21で作成したテーブル要素に追加で年齢のソート機能を追加しました。
ソートボタンの追加
今回ソートボタンの追加方法を課題21から変更しました。レビューした際にみたオブジェクトを使用してるのがいいなあと思い、真似させていただきました。
まずはオブジェクトに、ソートの有無の情報(hasSort)を追加。今回はIDと年齢箇所をtrueに設定。
それぞれのhasSortの値がtrueだった場合、ボタンを追加する。
const userDataColumn = {
userId: {
value : "ID",
hasSort : true
},
name: {
value : "名前",
hasSort : false
},
gender: {
value : "性別",
hasSort : false
},
age: {
value : "年齢",
hasSort : true
}
}
const getTableHeadElement = () => {
const thead = createElementWithClassName("thead", "thead");
const tr = createElementWithClassName("tr", "tr");
const fragmentThElement = document.createDocumentFragment();
//userDataColumnオブジェクトのキーを取得
const userDataColumnKeys = Object.keys(userDataColumn);
userDataColumnKeys.forEach((key) => {
const th = createElementWithClassName("th", "th");
const value = userDataColumn[key].value;
th.textContent = value;
//userDataColumnオブジェクトのhasSortがtrueだったらボタンを追加する
userDataColumn[key].hasSort && th.appendChild(createSortButton());
fragmentThElement.appendChild(tr).appendChild(th);
});
thead.appendChild(fragmentThElement);
return thead;
}レビューPOINT
元々のオブジェクトの「hasSort:」については、単純に「Sort:」という名前を使用していましたが、「has***」を使用することでboolean値を含むことを示すことができるとレビューいただきました。
塾の記事共有部屋にも貼ってありました下記参考記事を読みましたが、その時の使い方によって命名については細かく分けた方が良さそうです。今後はそちらも意識しようと思いました、勉強になります。安易にネーミングしすぎなところは改善点です。
クリックイベントの付与
今まで１つにまとめていた、クリックイベントについての関数を、今回
①ソートボタンへのクリックイベントの付与
②クリックによる処理
とで分けました。その２つが一緒になっていることで可読性を下げているのではとご指摘いただき、確かに分けたらもっとクリアになりそうと思い修正しました。
しかしながら、この２つの仕事を分けたことによって、引数の渡し方が冗長になってしまいました。
（自分は安易に、処理に必要な要素をそのまま引数として渡していた）
const addClickEventToSortButton = () => {
//レビューPOINT③
const sortButtons = [...document.querySelectorAll(".js-sortButton")];
const trElement = [...document.querySelectorAll("tbody > tr")];
sortButtons.forEach((sortButton) => {
sortButton.addEventListener("click", {
//レビューPOINT①、②
sortButtons,
trElement,
handleEvent: clickSortButton
});
});
};
function clickSortButton(event) {
const tbody = document.querySelector("tbody");
resetSortbuttonStatus(this.sortButtons, event.target);
const clickedCellIndex = event.target.parentElement.cellIndex;
const nextStatus = switchSortStatus(event.target);
event.target.dataset.status = nextStatus;
const sortedRows = sortFunc(event.target, clickedCellIndex, this.trElement);
while (tbody.firstChild) {
tbody.removeChild(tbody.firstChild);
}
sortedRows.forEach((row) => {
tbody.appendChild(row);
});
}
//１つにまとまっていた元のコード
// const sortButtonClickEvent = () => {
// const tbody = document.querySelector('tbody');
// const sortButtons = [...document.querySelectorAll(".sortButton")];
// const trElement = [...document.querySelectorAll("tbody > tr")];
// sortButtons.forEach((sortButton)=> {
// sortButton.addEventListener('click', (e) => {
// resetSortbuttonStatus(sortButtons, e.target);
// const clickedCellIndex = e.target.parentElement.cellIndex;
// const nextStatus = switchSortStatus(e.target);
// sortButton.dataset.status = nextStatus;
// const sortedRows = sortFunc(e.target,clickedCellIndex,trElement);
// while (tbody.firstChild) {
// tbody.removeChild(tbody.firstChild);
// }
// sortedRows.forEach((row) => {
// tbody.appendChild(row);
// });
// });
// })
// }レビューPOINT①
関数の引数が多い場合、Patterns Objectを利用するのが良いと教えていただきました。
”関数の引数をオブジェクトで定義することにより、呼び出し元にプロパティ名をつけられます。このようにすると、呼び出し元を見たときに引数に必要な値が分かりやすいですし、プロパティの順番を間違えずに済みます。”
参考：JavaScriptのコードには、こんな書き方もある
引数がいくつも渡ってくることによって、何の引数が渡ってきたのか呼び出し元を見ただけでは判断しづらく可読性が低いです。オブジェクトで定義することで、わかりやすくなります。また、オブジェクトで指定する場合、handleEventにて関数の定義をすることについても勉強になりました。
レビューPOINT②
また、その引数で渡していたオブジェクトを省略できるのでは、とアイディアいただきました。
なんと、そんなことができたのですね、勉強になります… 知らないことばかり。みんないろんな記事読んでるんだろうな。。
ここは御二方の意見を取り入れたことによって、とっても良くなりました！
レビューPOINT③
ソートボタンのクラスについて、js- プレフィックスをつけた方がいいのではとレビューいただきました。JSで操作してるのと同時に、CSSも同じクラス名でスタイルをあててしまっているので、バグが起きる可能性があるのではないかとご指摘いただきました。そういった視点も甘かったなと感じました、今後に活かします！細かいところまで見ていただき感謝です。
ボタンリセット
片方のボタンが押された際、もう片方のボタンのステータスをリセットする必要があるため、下記関数を追加しました。
const resetSortbuttonStatus = (sortButtons, clickedButton) => {
sortButtons.forEach((sortButton) => {
if (sortButton === clickedButton) return;
sortButton.dataset.status = Sort.Default;
});
};
//元のコード
// const resetSortbuttonStatus = (sortButtons, target) => {
// sortButtons.forEach((button)=> {
// if (button !== target) {
// button.dataset.status = Sort.Default;
// }
// })
// }レビューPOINT
元のコードを見るとわかるのですが、もう少し具体的に説明したほうがわかりやすいのではとレビューいただきました。引数名が安易ですね。。アドバイスいただいたコードを見るととてもわかりやすいです。if文はネストが増えることで可読性が下がるので、早期リターンで書くことでスッキリしてます。
ソート箇所の変更
当初は、ソートの切り替えをif文で組んでいましたが、ここはswitch文でも書けるとアイディアいただきました。
switch文の方が記述も少なく、わかりやすいと感じたのでここはswitch文に修正しました。
また、そのソートボタンのdata-属性(ステータス)の切り替えについて、デフォルト値の設定はswitch文の”default”に指定できないかとアイディアいただきました。特に動きに問題はなさそうなので、記述量が少ないこちらを採用させていただきました。
const Sort = {
Default: "default",
Asc: "asc",
Desc: "desc"
};
const switchSortStatus = (target) => {
switch (target.dataset.status) {
case Sort.Asc:
return Sort.Desc;
case Sort.Desc:
return Sort.Default;
default:
return Sort.Asc;
}
}
//元のコード
// const switchSortStatus = (target) => {
// switch (target.dataset.status) {
// case Sort.Default:
// return Sort.Asc;
// case Sort.Asc:
// return Sort.Desc;
// case Sort.Desc:
// return Sort.Default;
// default:
// return Sort.defaul;
// }
// }
//switch文に変更
const sortFunc = (target,clickedCellIndex,defaultRows) => {
const defaultTrElement = [...defaultRows];
const currentStatus = target.dataset.status;
switch (currentStatus) {
case Sort.Asc:
return defaultTrElement.sort((a, b) => a.children[clickedCellIndex].textContent - b.children[clickedCellIndex].textContent);
case Sort.Desc:
return defaultTrElement.sort((a, b) => b.children[clickedCellIndex].textContent - a.children[clickedCellIndex].textContent);
default:
return defaultRows;
}
}
//元のコード
// const sortFunc = (target,clickedCellIndex,defaultRows) => {
// const defaultTrElement = [...defaultRows];
// const currentStatus = target.dataset.status;
// if (currentStatus === Sort.Default) {
// return defaultRows;
// }
// if (currentStatus === Sort.Asc){
// return defaultTrElement.sort((a,b) => a.children[clickedCellIndex].textContent - b.children[clickedCellIndex].textContent);
// }
// if (currentStatus === Sort.Desc){
// return defaultTrElement.sort((a,b) => b.children[clickedCellIndex].textContent - a.children[clickedCellIndex].textContent);
// }
// }後記
皆様の意見を取り入れて参考にさせていただいたことによって、とてもいいコードになった気がして嬉しい。色々勉強させていただきました。学んで、次からは自分でも思いつくようになりたい。一旦はテーブル操作課題は完、次に進みます。
c.sakyou