マジックナンバーを排除して、保守性の高いコードを書きたいです。前回も保守性を高めたいと質問させていただきお世話になりました。
例えば、
<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の開発環境の時、文字列の状態のコードだと検出できなさそうなので、どうにかして、プログラムの文字列表現から抜け出したいです。
よろしくお願いします。
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>
ご回答ありがとうございます。以前にもお世話になりました
コードが分かりやすいです。tableはどこに書くを自由にしたいので、Ogre7077さんのコードを少し変えてappendTableTo()をelementを返すメソッドにしたりしようとかなと思います。ありがとうございました!
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>
ご回答ありがとうございます。いつもお世話になっております。
やっとDOMの良さがわかりました!これなら私の望み通りにコードが組めそうです。少しきになるのがこのコードだと、他にTABLEがあった時にも反応してしまうことです。handle()のifの条件を変えれば対応できるのかもしれませんが…
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 で動作確認しました。
ご回答ありがとうございます
タグの部分をidだけにして、書いてあとでイベントリスナーを追加する発想はなかったです!ですが、この方法だと引数が複数になった時に困るので、いつも使える解決法ではないのかなと思いました。ありがとうございます
お探しのQ&Aが見つからない時は、教えて!gooで質問しましょう!
関連するカテゴリからQ&Aを探す
おすすめ情報
- ・漫画をレンタルでお得に読める!
- ・街中で見かけて「グッときた人」の思い出
- ・「一気に最後まで読んだ」本、教えて下さい!
- ・幼稚園時代「何組」でしたか?
- ・激凹みから立ち直る方法
- ・1つだけ過去を変えられるとしたら?
- ・【あるあるbot連動企画】あるあるbotに投稿したけど採用されなかったあるある募集
- ・【あるあるbot連動企画】フォロワー20万人のアカウントであなたのあるあるを披露してみませんか?
- ・映画のエンドロール観る派?観ない派?
- ・海外旅行から帰ってきたら、まず何を食べる?
- ・誕生日にもらった意外なもの
- ・天使と悪魔選手権
- ・ちょっと先の未来クイズ第2問
- ・【大喜利】【投稿~9/7】 ロボットの住む世界で流行ってる罰ゲームとは?
- ・推しミネラルウォーターはありますか?
- ・都道府県穴埋めゲーム
- ・この人頭いいなと思ったエピソード
- ・準・究極の選択
- ・ゆるやかでぃべーと タイムマシンを破壊すべきか。
- ・歩いた自慢大会
- ・許せない心理テスト
- ・字面がカッコいい英単語
- ・これ何て呼びますか Part2
- ・人生で一番思い出に残ってる靴
- ・ゆるやかでぃべーと すべての高校生はアルバイトをするべきだ。
- ・初めて自分の家と他人の家が違う、と意識した時
- ・単二電池
- ・チョコミントアイス
デイリーランキングこのカテゴリの人気デイリーQ&Aランキング
-
<JavaScript>tableタグを入力不...
-
画面表示とともに、テーブルの...
-
テーブルの生成
-
selectのonChangeが動作しません
-
javascriptで表に画像を貼る
-
return trueとreturn falseの用...
-
プルダウン選択を変更すると、...
-
チェックボックス付きのテーブ...
-
value内に変数を入れたい
-
【javascript・PHP】プルダウン...
-
JSPとJavaScriptの連携について...
-
連動プルダウンのclonenode
-
チェックボックスのON/OFFでVal...
-
同名ボタンのクリック時要素番...
-
複数のプルダウンを1つにまとめ...
-
新しくフォルダを作成したい
-
ラジオボタンと連動して文字列...
-
FormのonsubmitでJavaスクリプ...
-
javascriptでhiddenに二次元配...
-
確認ダイアログの出し方(JavaS...
マンスリーランキングこのカテゴリの人気マンスリーQ&Aランキング
-
<JavaScript>tableタグを入力不...
-
テーブルの行数を可変長にした...
-
selectのonChangeが動作しません
-
画面表示とともに、テーブルの...
-
javascriptで<table>背景色の取得
-
javascriptでクリックするごと...
-
テキストエリアに入力した改行...
-
【jQuery】tableループ内のIDの...
-
<iframe>内にHTMLをランダム表...
-
javascriptで画像をテーブルに...
-
WEB制作に関する質問です。コン...
-
javascript でテーブル操作
-
カレンダーに印を付けたい
-
日にち指定によるテーブル/行の...
-
テーブルのtdの中に、重複避け...
-
innerHTMLでのテーブル作成
-
プルダウンメニューを表の中に...
-
簡単なJavaスロットマシーンに...
-
javascriptで表に画像を貼る
-
javascriptで画像の移動
おすすめ情報