回答編集履歴
3
テスト用コードの記述が混じっていたため、削除
answer
CHANGED
|
@@ -118,8 +118,7 @@
|
|
|
118
118
|
* @const
|
|
119
119
|
* @type {string}
|
|
120
120
|
*/
|
|
121
|
-
|
|
121
|
+
var LS_KEY = 'txt_todo';
|
|
122
|
-
var LS_KEY = 'teratail.15262.txt_todo.2';
|
|
123
122
|
|
|
124
123
|
/**
|
|
125
124
|
* ToDoに項目を追加する関数
|
2
空、もしくはスペースのみの項目の追加を防ぐ処理を追加。またそれに伴い、改善点の位置を変更。
answer
CHANGED
|
@@ -60,7 +60,19 @@
|
|
|
60
60
|
(上記の修正はJavaScriptに対してのみ行いました。このため、上記のコードはJavaScriptのみとなります)
|
|
61
61
|
|
|
62
62
|
動作も確認しています。
|
|
63
|
-
ただし、このコードには改善すべき点が数多くあ
|
|
63
|
+
ただし、このコードには以下に挙げる改善すべき点が数多くあります。
|
|
64
|
+
|
|
65
|
+
* ToDoは項目リストなのだから、`ul`、`li`要素を利用するべき
|
|
66
|
+
* 項目の入力値が空、もしくはスペースのみの場合、追加を行わない方が利便性が向上する。
|
|
67
|
+
* `onXXX`等の属性を利用してイベントを登録する方法は、イベントを重複して登録できない等の問題からあまり使用するべきではない。
|
|
68
|
+
(このコードでは重複登録はないが、利用者のアドオン等がそのような実装になっている場合も。その場合、利用者のアドオンが正常に動作しない可能性が考えられる)
|
|
69
|
+
[addEventListener](https://developer.mozilla.org/ja/docs/Web/API/EventTarget/addEventListener)(Internet Explorer 8以前に対応するなら、[attachEvent](https://developer.mozilla.org/ja/docs/Web/API/EventTarget/addEventListener#Compatibility)も併用)を利用したほうが良い。
|
|
70
|
+
* `innerHTML`は重い。このため、[document.createElement](https://developer.mozilla.org/ja/docs/Web/API/Document/createElement)等でHTMLに対応するDOMを生成するか、[insertAdjacentHTML](https://developer.mozilla.org/ja/docs/Web/API/Element/insertAdjacentHTML)を利用した方が良い。
|
|
71
|
+
* `window.localStorage`オブジェクトは全てのブラウザが対応しているわけではない。`window.localStorage`を利用する処理を行う前に、これに対応しているか検証するべき。
|
|
72
|
+
* localStorageにはHTMLを直接保存するより、JavaScript側で配列を作成し、その配列に項目の情報を入れ、それを[JSON.stringify](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify)で[JSON](https://ja.wikipedia.org/wiki/JavaScript_Object_Notation)に変換し、保存したほうが良い。これにより、利用者のハードディスクの使用量を軽減できる。
|
|
73
|
+
なお、[JSON](https://ja.wikipedia.org/wiki/JavaScript_Object_Notation)から配列に戻す場合は[JSON.parse](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse)を使用。
|
|
74
|
+
|
|
75
|
+
このため、私ならば以下のようにコードを書きます。
|
|
64
76
|
(下記コードはInternet Explorer 8以前には対応していません。対応する必要がある場合はコメントをお願いします。)
|
|
65
77
|
|
|
66
78
|
```HTML
|
|
@@ -106,35 +118,49 @@
|
|
|
106
118
|
* @const
|
|
107
119
|
* @type {string}
|
|
108
120
|
*/
|
|
109
|
-
var LS_KEY = 'txt_todo';
|
|
121
|
+
//var LS_KEY = 'txt_todo';
|
|
122
|
+
var LS_KEY = 'teratail.15262.txt_todo.2';
|
|
110
123
|
|
|
111
124
|
/**
|
|
112
125
|
* ToDoに項目を追加する関数
|
|
113
126
|
*/
|
|
114
127
|
function add_todo() {
|
|
115
|
-
/**
|
|
116
|
-
* 入力欄の入力値を取得し、Stringオブジェクトに変換
|
|
117
|
-
*
|
|
118
|
-
* Note: ここでStringオブジェクトに変換しているのは、重複した値を持つ項目を識別するため
|
|
119
|
-
* プリミティブ値(普通の文字列)では同じ値の項目を区別できないが、
|
|
120
|
-
* オブジェクトは区別できる
|
|
121
|
-
|
|
128
|
+
var value;
|
|
122
|
-
var item_value
|
|
129
|
+
var item_value;
|
|
123
130
|
|
|
124
131
|
/**
|
|
125
|
-
*
|
|
132
|
+
* 入力欄の入力値を取得
|
|
126
133
|
*/
|
|
127
|
-
|
|
134
|
+
var value = document.getElementById('txt_todo').value;
|
|
128
135
|
|
|
129
136
|
/**
|
|
130
|
-
*
|
|
137
|
+
* 入力値が空、あるいはスペースのみの場合、項目を追加しない
|
|
131
138
|
*/
|
|
139
|
+
if (value.trim() !== '') {
|
|
140
|
+
/**
|
|
141
|
+
* 入力欄の入力値をStringオブジェクトに変換
|
|
142
|
+
*
|
|
143
|
+
* Note: ここでStringオブジェクトに変換しているのは、重複した値を持つ項目を識別するため
|
|
144
|
+
* プリミティブ値(普通の文字列)では同じ値の項目を区別できないが、
|
|
145
|
+
* オブジェクトは区別できる
|
|
146
|
+
*/
|
|
132
|
-
|
|
147
|
+
item_value = new String(value);
|
|
133
148
|
|
|
134
|
-
|
|
149
|
+
/**
|
|
150
|
+
* ToDoの情報を配列に追加
|
|
151
|
+
*/
|
|
152
|
+
todo_items.push(item_value);
|
|
153
|
+
|
|
154
|
+
/**
|
|
155
|
+
* HTML内に要素を追加する
|
|
156
|
+
*/
|
|
157
|
+
add_item_elem(item_value);
|
|
158
|
+
|
|
159
|
+
/**
|
|
135
|
-
|
|
160
|
+
* データを保存
|
|
136
|
-
|
|
161
|
+
*/
|
|
137
|
-
|
|
162
|
+
save();
|
|
163
|
+
}
|
|
138
164
|
}
|
|
139
165
|
|
|
140
166
|
/**
|
|
@@ -222,7 +248,7 @@
|
|
|
222
248
|
if (todo_json_data) {
|
|
223
249
|
try {
|
|
224
250
|
/**
|
|
225
|
-
* localStorageから読み込んだJSONを
|
|
251
|
+
* localStorageから読み込んだJSONをJavaScriptオブジェクトに変換
|
|
226
252
|
*/
|
|
227
253
|
todo_saved_data = JSON.parse(todo_json_data);
|
|
228
254
|
|
|
@@ -280,17 +306,4 @@
|
|
|
280
306
|
</script>
|
|
281
307
|
</body>
|
|
282
308
|
</html>
|
|
283
|
-
```
|
|
284
|
-
|
|
285
|
-
改善点として、
|
|
286
|
-
|
|
287
|
-
* ToDoは項目リストなのだから、`ul`、`li`要素を利用するべき
|
|
288
|
-
* `onXXX`等の属性を利用してイベントを登録する方法は、イベントを重複して登録できない等の問題からあまり使用するべきではない。
|
|
289
|
-
(このコードでは重複登録はないが、利用者のアドオン等がそのような実装になっている場合も。その場合、利用者のアドオンが正常に動作しない可能性が考えられる)
|
|
290
|
-
[addEventListener](https://developer.mozilla.org/ja/docs/Web/API/EventTarget/addEventListener)(Internet Explorer 8以前に対応するなら、[attachEvent](https://developer.mozilla.org/ja/docs/Web/API/EventTarget/addEventListener#Compatibility)も併用)を利用したほうが良い。
|
|
291
|
-
* `innerHTML`は重い。このため、[document.createElement](https://developer.mozilla.org/ja/docs/Web/API/Document/createElement)等でHTMLに対応するDOMを生成するか、[insertAdjacentHTML](https://developer.mozilla.org/ja/docs/Web/API/Element/insertAdjacentHTML)を利用した方が良い。
|
|
292
|
-
* `window.localStorage`オブジェクトは全てのブラウザが対応しているわけではない。`window.localStorage`を利用する処理を行う前に、これに対応しているか検証するべき。
|
|
293
|
-
* localStorageにはHTMLを直接保存するより、JavaScript側で配列を作成し、その配列に項目の情報を入れ、それを[JSON.stringify](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify)で[JSON](https://ja.wikipedia.org/wiki/JavaScript_Object_Notation)に変換し、保存したほうが良い。これにより、利用者のハードディスクの使用量を軽減できる。
|
|
294
|
-
なお、[JSON](https://ja.wikipedia.org/wiki/JavaScript_Object_Notation)から配列に戻す場合は[JSON.parse](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse)を使用。
|
|
295
|
-
|
|
296
|
-
が挙げられます。
|
|
309
|
+
```
|
1
冒頭の文章で、伝わりにくいと判断した表現に加筆
answer
CHANGED
|
@@ -1,8 +1,8 @@
|
|
|
1
1
|
修正を可能な限り少なくするならば、以下の様なコードとなります。
|
|
2
2
|
他の修正として、
|
|
3
3
|
|
|
4
|
-
* localStorageに対応するデータが
|
|
4
|
+
* localStorageに対応するデータが保存されていない(初回利用時の)場合が想定できていない
|
|
5
|
-
* 削除した時に保存されていない
|
|
5
|
+
* 項目を削除した時にデータが保存されていない
|
|
6
6
|
|
|
7
7
|
この2点も修正しました。
|
|
8
8
|
|