JavaScript 保守性の高めたい 2

解決済

質問者：newtgecko
質問日時：2014/05/21 23:31
回答数：3件

マジックナンバーを排除して、保守性の高いコードを書きたいです。前回も保守性を高めたいと質問させていただきお世話になりました。

例えば、
<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の開発環境の時、文字列の状態のコードだと検出できなさそうなので、どうにかして、プログラムの文字列表現から抜け出したいです。

よろしくお願いします。

通報する

この質問への回答は締め切られました。

質問の本文を隠す

回答 (3件)

ベストアンサー優先
最新から表示
回答順に表示

No.3ベストアンサー

回答者： Ogre7077
回答日時：2014/05/22 11:45

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

- 0
- 件

通報する

この回答へのお礼

ご回答ありがとうございます。以前にもお世話になりました

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

通報する

お礼日時：2014/05/23 09:41

No.2

回答者： babu_baboo
回答日時：2014/05/22 02:29

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>

- 0
- 件

通報する

この回答へのお礼

ご回答ありがとうございます。いつもお世話になっております。

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

通報する

お礼日時：2014/05/23 09:35

No.1

回答者： hitomura
回答日時：2014/05/22 01:42

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