- ベストアンサー
JavaScriptコードの保守性を高めるためのマジックナンバーの排除方法
- JavaScriptコードの保守性を上げるために、マジックナンバーを排除する方法を紹介します。
- マジックナンバーを文字列で表現すると、変更し忘れのリスクがあります。リファクタリング機能が使えない場合でも対応可能です。
- マジックナンバーを変数で置き換えることで、プログラムの可読性を高め、保守性を向上させることができます。
- newtgecko
- お礼率88% (150/169)
- JavaScript
- 回答数3
- ありがとう数3
- みんなの回答 （3）
- 専門家の回答
質問者が選んだベストアンサー
> プログラムの文字列表現から抜け出したい document.write → 親要素.appendChild <td onclick="処理"> → td要素.addEventListener('click',処理,false) 以下サンプルです <div id=kokoniTableKake></div> <script>(function(){ var _; _ ; var User = function (name, age) { _ , _ ; this.name = name; _ , _ ; this.age = age; _ , _ ; this.show = function(){ alert(this.name +" ("+ this.age+")") }; _ ; } _ ; var UsersTableBuilder = function(users) { _ , _ ; this.makeListener = function(user) { return function(event){user.show()} }; _ , _ ; this.appendTableTo = function(id) { _ , _ , _ ; var tab = document.createElement('table'); _ , _ , _ ; tab.setAttribute('border', true); _ , _ , _ ; for (var uid in users) { _ , _ , _ , _ ; var td = document.createElement('td'); _ , _ , _ , _ ; var tr = document.createElement('tr'); _ , _ , _ , _ ; tab.appendChild(tr).appendChild(td); _ , _ , _ , _ ; var listener = this.makeListener(users[uid]); _ , _ , _ , _ ; td.addEventListener('click', listener, false); // onclick の代替 _ , _ , _ , _ ; td.textContent = uid; _ , _ , _ ; } _ , _ , _ ; document.getElementById(id).appendChild(tab); // write の代替 _ , _ ; }; _ ; }; _ ; var users = { NkxyZ: new User("Taro", 41), BkcSk: new User("Hanako", 17), HnViOj: new User("Harushige", 8) }; _ ; var builder = new UsersTableBuilder(users); _ ; builder.appendTableTo('kokoniTableKake'); })()</script>
その他の回答 (2)
- babu_baboo
- ベストアンサー率51% (268/525)
UsersTableBuilderをオブジェクトにする意味が消え失せています。 （唯一であるIDが変更できなく・・・） 最初のアドバイスで、dom を使っては？というアドバイスがありましたよね。 個々の要素にaddEventListenerするのは・・・ <!DOCTYPE html> <meta charset="utf-8"> <title></title> <style> table, td { border:blue 1px solid; } td:nth-of-type(1) { display : none; } </style> <body> <script> var User = function (name, age) { this.name = name; this.age = age; } User.prototype.show = function(){ alert(this.name +" ("+ this.age+")"); } var users = { 'NkxyZ': new User("Taro", 41), 'BkcSk': new User("Hanako", 17), 'HnViOj': new User("Harushige", 8) }; function key_name_age (users) { var result = []; for (var prop in users) if (users.hasOwnProperty (prop)) result.push ([prop, users[prop].name, users[prop].age]); return result; } var createTbody = function (insertTR) { return function (ary) { var tbody = document.createElement ('tbody'); ary.forEach (insertTR, tbody); return tbody; }; }(function (insertTD) { return function (record) { record.forEach (insertTD, this.insertRow (-1)); }; }(function (text) { this.insertCell (-1).textContent = text; })); function callBack (id) { var obj = users[id]; !obj || obj.show (); } function handle (event) { var e = event.target; var th, id; if ('TD' === e.tagName) { th = e.parentNode.querySelector ('td'); id = th.firstChild.nodeValue; callBack (id); } } document.body.appendChild ( document.createElement ('table').appendChild ( createTbody ( key_name_age (users)))); document.addEventListener ('click', handle, false); </script>
お礼
ご回答ありがとうございます。いつもお世話になっております。 やっとDOMの良さがわかりました！これなら私の望み通りにコードが組めそうです。少しきになるのがこのコードだと、他にTABLEがあった時にも反応してしまうことです。handle()のifの条件を変えれば対応できるのかもしれませんが…
- hitomura
- ベストアンサー率48% (325/664)
for(var id in users){ out += "<tr><td onclick=\"users['"+ id +"'].show();\">User</td></tr>" } を for(var id in users){ out += "<tr><td id=\""+ id +"\">User</td></tr>" } として、メインの最後に var tds = document.getElementsByTagName("td"); for (var i = 0; i < tds.length; i++){ tds[i].addEventListener("click", function(){ users[this.id].show(); }); } を追加するというのはどうでしょうか。一応 IE 11 で動作確認しました。
お礼
ご回答ありがとうございます タグの部分をidだけにして、書いてあとでイベントリスナーを追加する発想はなかったです！ですが、この方法だと引数が複数になった時に困るので、いつも使える解決法ではないのかなと思いました。ありがとうございます
お礼
ご回答ありがとうございます。以前にもお世話になりました コードが分かりやすいです。tableはどこに書くを自由にしたいので、Ogre7077さんのコードを少し変えてappendTableTo()をelementを返すメソッドにしたりしようとかなと思います。ありがとうございました！