JavaScriptコードの保守性を高めるためのマジックナンバーの排除方法

2014/05/21 23:31

このQ&Aのポイント

JavaScriptコードの保守性を上げるために、マジックナンバーを排除する方法を紹介します。
マジックナンバーを文字列で表現すると、変更し忘れのリスクがあります。リファクタリング機能が使えない場合でも対応可能です。
マジックナンバーを変数で置き換えることで、プログラムの可読性を高め、保守性を向上させることができます。

JavaScript 保守性の高めたい 2

マジックナンバーを排除して、保守性の高いコードを書きたいです。前回も保守性を高めたいと質問させていただきお世話になりました。例えば、 <table border="1"> 　<tr> 　　<td onclick="users['NkxyZ'].show();">User</td> 　</tr> 　<tr> 　　<td onclick="users['BkcSk'].show();">User</td> 　</tr> 　<tr> 　　<td onclick="users['HnViOj'].show();">User</td> 　</tr> </table> 上記のHTMLを生成する、以下のコードがあります <script> // メイン　var users = {'NkxyZ': new User("Taro", 41), 　　　　　　　　　'BkcSk': new User("Hanako", 17), 　　　　　　　　　'HnViOj': new User("Harushige", 8)}; 　var usersTable = new UsersTableBuilder(); 　document.write(usersTable.getTable(users)); </script> <script> // 以下のコードは外部ファイルで、読み込み済みです　// ユーザを管理するクラス　var User = function(name, age){ 　　this.name = name; 　　this.age = age; 　} 　User.prototype.show = function(){ 　　alert(this.name +" ("+ this.age+")"); 　} 　　// ユーザテーブルを返すクラス　var UsersTableBuilder = function(){ 　　this.tagId = 'users_table'; 　} 　UsersTableBuilder.prototype.getTable = function(users){ 　　var out = "<table border='1'>"; 　　for(var id in users){ 　　　out += "<tr><td onclick=\"users['"+ id +"'].show();\">User</td></tr>" 　　} 　　out += "</table>"; 　　return out; 　} </script> このコードで私が一番気に入らないのは、UsersTableBuilderのgetTable()メソッドです。このメソッドのfor文内の　"<tr><td onclick=\"users['"+ id +"'].show();\">User</td></tr>"　では User#show()メソッドが文字列で表現されてしまっています。show()がマジックナンバーぽくなってしまっています。このままでは、Userクラスでshow()メソッドの名前を変えた時などに、変更し忘れになるかもしれません。 JavaのEclipseには変数名を変更できるリファクタリング機能がありますが、そのような機能が使えるJavaScriptの開発環境の時、文字列の状態のコードだと検出できなさそうなので、どうにかして、プログラムの文字列表現から抜け出したいです。よろしくお願いします。

newtgecko
お礼率88% (150/169)

JavaScript
回答数3
ありがとう数3

回答全件

ベストアンサー

> プログラムの文字列表現から抜け出したい document.wri…

2014/05/22 11:45

UsersTableBuilderをオブジェクトにする意味が消え失せて…

2014/05/22 02:29

　　for(var id in users){ 　　　out += "…

2014/05/22 01:42

みんなの回答 （3）
専門家の回答

質問者が選んだベストアンサー

ベストアンサー

Ogre7077
ベストアンサー率65% (170/258)

2014/05/22 11:45 回答No.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>

質問者

お礼 2014/05/23 09:41

ご回答ありがとうございます。以前にもお世話になりましたコードが分かりやすいです。tableはどこに書くを自由にしたいので、Ogre7077さんのコードを少し変えてappendTableTo()をelementを返すメソッドにしたりしようとかなと思います。ありがとうございました！

全文を見る

ログインすると、全ての回答が全文表示されます。

その他の回答 (2)

babu_baboo
ベストアンサー率51% (268/525)

2014/05/22 02:29 回答No.2

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>

質問者

お礼 2014/05/23 09:35

ご回答ありがとうございます。いつもお世話になっております。やっとDOMの良さがわかりました！これなら私の望み通りにコードが組めそうです。少しきになるのがこのコードだと、他にTABLEがあった時にも反応してしまうことです。handle()のifの条件を変えれば対応できるのかもしれませんが…

全文を見る

ログインすると、全ての回答が全文表示されます。

hitomura
ベストアンサー率48% (325/664)

2014/05/22 01:42 回答No.1

　　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 で動作確認しました。

質問者

お礼 2014/05/23 09:27

ご回答ありがとうございますタグの部分をidだけにして、書いてあとでイベントリスナーを追加する発想はなかったです！ですが、この方法だと引数が複数になった時に困るので、いつも使える解決法ではないのかなと思いました。ありがとうございます

全文を見る

ログインすると、全ての回答が全文表示されます。

関連するQ&A

javascript でテーブル操作
javascriptのdeleteRowで削除したテーブルの行を再表示する方法を探しております。下記ソース内reload.gifをクリックするとdeleteRowで消えた行をまとめて表示したいのですが何か良い方法は無いでしょうか？ (＊)ブラウザの再表示は使えません。 <html> <head> <title></title> <script language="JavaScript" type="text/JavaScript"></script> </head> <body> <img src="reload.gif"> <table id="table1"> <tr onmouseover="asrow1 = this.rowIndex"> <td class="ChkTD41">ああ<img src="del.gif" onclick="Delline('table1',asrow1)" /> <input type="text" name="a1"> </td> </tr> <tr onmouseover="asrow2 = this.rowIndex"> <td class="ChkTD41">いい<img src="del.gif" onclick="Delline('table1',asrow2)" /> <input type="text" name="b1"> </td> </tr> <tr onmouseover="asrow3 = this.rowIndex"> <td>うう<img src="del.gif" onclick="Delline('table1',asrow3)" /> <input type="text" name="c1"> </td> </tr> </table> </body> </html>
- ベストアンサー
- JavaScript
javascriptの変数の渡し方
ラジオボタンで選択した値によって表示するテーブルを変えるフォームを作っています。 AAAは入力ボックスが1つ BBBは入力ボックスが2つ同じphpへ送信して、入力ボックスへデータは入るのですが、javascriptの値を渡すことが出来ません。 BBBでsubmit後、BBBの入力ボックスを表示することは出来ますでしょうか。 <? echo "<form method=post action=?>"; echo "<input type=radio name='user' onclick='change_user(this.value)' uid='user' value=1"; if($user==1){ echo " checked"; } echo ">AAA"; echo "<input type=radio name='user' onclick='change_user(this.value)' uid='user' value=2"; if($user==2){ echo " checked"; } echo ">BBB"; echo "<br>"; echo "<span id=inner>"; echo "<table>"; echo "<tr>"; echo "<td>AAAのフォーム</td>"; echo "<td><input name=form1 type=text value=$form1></td>"; echo "</tr>"; echo "</table>"; echo "</span>"; echo "<span id=outer>"; echo "<table>"; echo "<tr>"; echo "<td>BBBのフォーム</td>"; echo "<td><input name=form1 type=text value=$form1></td>"; echo "<td><input name=form2 type=text value=$form2></td>"; echo "</tr>"; echo "</table>"; echo "</span>"; echo "<br>"; echo "<input type=submit value=submit>"; echo "</form>"; ?> <script language='javascript'> var inner = document.getElementById('inner'); var outer = document.getElementById('outer'); function change_user (uid) { if (uid == '2') { inner.style.display = 'none'; outer.style.display = ''; } else { inner.style.display = ''; outer.style.display = 'none'; } } change_user(document.getElementById('user').value); </script>
- ベストアンサー
- PHP
javascript オセロ
javascriptでオセロを作ろうと思っています。初めに「クリックしたtdタグの背景色を、クリック数が奇数回なら白、偶数なら黒にする」コードを書きたいのですが、変数とthis(クリックされたtdタグ)の使い方がいまいち分かりません。ご教授いただければと思います。 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <title>無題ドキュメント</title> </head> <style type="text/css"> * { margin: 0; padding: 0; } td { width: 100px; height: 100px; } .green { background-color:#093; } .black { background-color:#333; } .white { background-color:#CCC; } </style> <body> <table> <tr> <td class="green" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> </tr> <tr> <td class="green" onClick="osero(this)"></td> <td class="black" onClick="osero(this)"></td> <td class="white" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> </tr> <tr> <td class="green" onClick="osero(this)"></td> <td class="white" onClick="osero(this)"></td> <td class="black" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> </tr> <tr> <td class="green" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> <td class="green" onClick="osero(this)"></td> </tr> </table> <script type="text/javascript">  </script> </body> </html> よろしくお願いします。
- ベストアンサー
- JavaScript
２つのjavascriptを１つにするには
教えてください。下記のフォームでチェックボックスにチェックを入れると　1.ラジオボタンがグレーアウトして選択不可能になる　2.ファイル選択ボタンがグレーアウトして選択不可能になる　3.ラジオボタンの選択が全てクリアされる　4.選択したファイルがクリアされるを同時に実現したいと思っています。 1～3は実現できているのですが、4を同時に実行することができていません。チェックボックスにチェックを入れただけで、クリアボタンを押さなくても【script(1)】と【script(2)】を同時に実行するにはどのようにすればいいでしょうか？よろしくお願いします。 <html> <head>  <script type="text/javascript">  </script>  <script type="text/javascript">  </script> </head> <body> <table border="1"> <tr><td> <p>画像2</p> <p><input type="checkbox" id="img2no" name="画像の掲載" value="なし" onclick="checkdis(this, ['img2a', 'img2b', 'img2c', 'img2d', 'img2e']);">該当なし</p> </td></tr> <tr><td>【掲載する画像の種類をお選び下さい。】 <ul> <li><input type="radio" id="img2a" name="画像2の種類" value="写真">写真</li> <li><input type="radio" id="img2b" name="画像2の種類" value="イラスト">イラスト</li> </ul> </td></tr> <tr><td>【画像の人物との関係をお選び下さい。】 <ul> <li><input type="radio" id="img2c" name="画像2の人物" value="本人">本人</li> <li><input type="radio" id="img2d" name="画像2の人物" value="ご家族">ご家族</li> </ul> </td></tr> <tr><td> 【ファイルをお選び下さい。】 <p align="center"> <span id="file2"><input type="file" name="画像2" id="img2e" size="45"></span> <span><input type="button" value="クリア" onclick="fileRefClear('file2'); return false;"></span> </p> </td></tr> </table> </body> </html>
- 締切済み
- JavaScript
JavaScript 保守性の高いコードの書き方
保守性の高いコードを書きたいです。JavaScriptでdocument.write("<div id='main'></div>")みたいに書いて、後でこの要素にいろいろなコードでdocument.getElementById('main').innerHTMLします。しかし、その後id名mainを他の名前に変えると、多くのソースコードを手作業で変更しなくてはなりません。この手間を削減するJavaScriptの書き方はありますか？例えば、 <table id="board"> 　　<tr> 　　　<td id="b0_0">A</td> 　　　<td id="b0_1">A</td> 　　　<td id="b0_2">A</td> 　　　<td id="b0_3">A</td> 　・　・　・上記のようなHTMLを作成する、以下のようなコードがあります。 <table id='board'></table> <script> 　var content = ""; 　for(i = 0; i < 5; i++){ 　　content += "<tr>" 　　for(j = 0; j < 4; j++){ 　　　content += "<td id='b"+ i + "_" + j +"'>A</td>" 　　} 　　content += "</tr>" 　} 　$('table#board').html(content) // JQueryを使っているつもりです　 </script> このコードの問題点は少なくとも2つあって、１つは、2重ループ内の「content += "<td id='b"+ i + "_" + j +"'>A</td>"」が読みづらいことです。 2つ目は、今は　td id='b0_1'のような形式ですが、このid名を変更しようと思ったときに、変更が容易ではないことです。特に2つ目は、いろいろな場所にあるコードから$('table#board td#b1_1').html('B')のような操作をしていると、かなり変更が大変です。こういったことを乗り越えるためのJavaScriptのコード設計法（？）やIDEやライブラリなどがありましたら教えて下さい。よろしくお願いします
- ベストアンサー
- JavaScript
JavaScriptとJSPの連携
JavaScriptのメソッドで受け取った値を JSPのスクリプトレットで配列番号として使いたいのですが認識してくれていないようです・・・。これは不可能なんでしょうか？ function setKey(no){ int colNo = parseInt(no); <% HashMap keys = （JSP部分で取得済みのHashMap）[colNo]; %> } ====== <TABLE><TR> <% for(int i; i < 配列.length; i++){ %> <TD> <INPUT　TYPE="button" NAME="btnSet<%= i %>" VALUE="セット" onClick="setKey('<%= i %>');" > ・・・・・・・・・・ </TD></TR> <% } %> <TABLE> こうすると、[colNo]を認識してくれないようなのですが・・・。
- ベストアンサー
- Java
表の一部のセルの背景色を変更したい
いつもお世話になっております。 IE6.0上で動作するものとして質問させていただきます。以下のソースの中でBODYタグのonloadイベントで initメソッドを呼びだし、メソッド中で表中の任意のセルの背景色を変更するにはどのようにすればよいのでしょうか。 --------------------------------------- <HTML> <HEAD> <script language="JavaScript">  </script> </HEAD> <BODY onload="init(); "> <FORM name="tttt"> <TABLE border="1" > 　<TBODY> 　　<TR> 　　　<TD onClick="this.style.backgroundColor = 'black';">test</TD> 　　　<TD onClick="this.style.backgroundColor = 'red';">test</TD> 　　　<TD onClick="this.style.backgroundColor = 'yellow';">test</TD> 　　</TR> 　</TBODY> </TABLE> </FORM> </BODY> </HTML> --------------------------------------- ソースは簡略化しておりますので、内容に不備がございましたら、ご容赦ください。ご回答をよろしくお願いいたします。
- ベストアンサー
- HTML
javascript文字をhtmlにセットする
恐れ入ります。 <script> function changeBoxColor( newColor ) { document.getElementById('irocolor').style.backgroundColor = newColor; } </script> <style> .b1{ background-color: #fffff0; } </style> <table border="1" id="irocolor"><tr><td> <a href="#" class="b1" onclick="changeBoxColor('#fffcf0');"></a> </td></tr></table> として、背景色の変更はできたのですが、この色コードを同一html内の<input type="hidden" name="color" value="XXXX">の中に代入したいのですが、方法が分かりません。よろしくお願い申し上げます。
- ベストアンサー
- JavaScript
javascriptで指定するtrを削除した時に、IEとfirefoxで動作が異なる
はじめて、投稿いたします。現在、javascriptで指定した行を削除するような動作を作っているのですが、firefoxとIEで動作が異なってしまうため困っています。ちなみに、firefoxでは、実現してほしい動作をしてくれるのですが、IEではうまく動作してくれません。 tableのtrが3つ以上ある場合は、指定した行が正常に削除します。しかし、IEだと、trが２つの時には、1番目のtrを削除しようとすると、一緒に2番目のtrの中身が見えなくなってしまいます。ちなみに、その中の情報が削除されたのかなと思い、alertで表示させるとどうやら削除はされておらず、表示しなくなってしまうようです。どうすれば、firefoxとIEで、同じような動作をするようにできるのでしょうか？よろしくお願いします。以下に、ソースを貼っておきます。 <html> <head> <script type="text/javascript">  </script> </head> <body> <h1>Hello World !!</h1> <table border="1" id="test_table"> <tr> <td> <input type="button" value="delete" onClick="delete_tr(this)" /> </td> <td style="display: none;">------ bbbb ------</td> <td>------ cccc ------</td> </tr> <tr> <td> <input type="button" value="delete" onClick="delete_tr(this)" /> </td> <td style="display: none;">------ BBBB ------</td> <td>------ CCCC ------</td> </tr> <tr align="center"> <td colspan="3"> <input type="button" value="add"> </td> </tr> </table> </body> </html>
- ベストアンサー
- JavaScript
JavaScriptの読み込み順
下記コードを実行すると、プルダウンメニューに値が入る前にボタンがクリックされてしまいます。どのようなコードを書けば正常に動作するのでしょうか。 ■html内 <html> <head> <title>SCT</title> <script type="text/javascript"> onload = function() { var d = new Date() ; d.setTime((new Date).getTime()) ; newOption = document.createElement('option') ; newOption.value = newOption.innerHTML = d.getFullYear()-1 ; document.getElementById('year').appendChild(newOption) ; newOption = document.createElement('option') ; newOption.value = newOption.innerHTML = d.getMonth()+1 ; document.getElementById('month').appendChild(newOption) ; newOption = document.createElement('option') ; newOption.value = newOption.innerHTML = d.getDate() ; document.getElementById('day').appendChild(newOption) ; } </script> <script type="text/javascript" language="JavaScript" src="./file.js"> </script> </head> <form NAME="Myfm" method="post" action="☆☆URL☆☆"> <body onLoad="yobi()"> <table border="0"> <tr> <td> <select name="startyear" id="year"> </select> </td> <td>年</td> <td> <select name="startmonth" id="month"> </select> </td> <td>月</td> <td> <select name="startday" id="day"> </select> </td> <td>日</td> </tr> <tr> <td> <select name="endyear"> <option value=201>2011 </select> </td> <td>年</td> <td> <select name="endmonth"> <option value=8>8 </select> </td> <td>月</td> <td> <select name="endday"> <option value=1>1 </select> </td> <td>日</td> </tr> <tr> <td><input type="submit" value="集計" NAME="mybut"> </td> </tr> </table> </body> </form> </html> ■file.js内 function yobi(){ document.Myfm.submit(); }
- ベストアンサー
- JavaScript

JavaScriptコードの保守性を高めるためのマジックナンバーの排除方法

JavaScript 保守性の高めたい 2