編集履歴

回答編集履歴

テスト用コードの記述が混じっていたため、削除

2015/09/03 18:53

投稿

スコア697

test CHANGED Viewed

@@ -238,9 +238,7 @@
      */
-    //var LS_KEY = 'txt_todo';
+    var LS_KEY = 'txt_todo';
-    var LS_KEY = 'teratail.15262.txt_todo.2';

空、もしくはスペースのみの項目の追加を防ぐ処理を追加。またそれに伴い、改善点の位置を変更。

2015/09/03 18:53

投稿

sounisi5011

スコア697

test CHANGED Viewed

@@ -122,7 +122,31 @@
 動作も確認しています。
-ただし、このコードには改善すべき点が数多くあるため、私ならば以下のようにします…
+ただし、このコードには以下に挙げる改善すべき点が数多くあります。
+* ToDoは項目リストなのだから、`ul`、`li`要素を利用するべき
+* 項目の入力値が空、もしくはスペースのみの場合、追加を行わない方が利便性が向上する。
+* `onXXX`等の属性を利用してイベントを登録する方法は、イベントを重複して登録できない等の問題からあまり使用するべきではない。
+  (このコードでは重複登録はないが、利用者のアドオン等がそのような実装になっている場合も。その場合、利用者のアドオンが正常に動作しない可能性が考えられる)
+  [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)も併用)を利用したほうが良い。
+* `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)を利用した方が良い。
+* `window.localStorage`オブジェクトは全てのブラウザが対応しているわけではない。`window.localStorage`を利用する処理を行う前に、これに対応しているか検証するべき。
+* 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)に変換し、保存したほうが良い。これにより、利用者のハードディスクの使用量を軽減できる。
+  なお、[JSON](https://ja.wikipedia.org/wiki/JavaScript_Object_Notation)から配列に戻す場合は[JSON.parse](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse)を使用。
+このため、私ならば以下のようにコードを書きます。
 (下記コードはInternet Explorer 8以前には対応していません。対応する必要がある場合はコメントをお願いします。)
@@ -214,7 +238,9 @@
      */
-    var LS_KEY = 'txt_todo';
+    //var LS_KEY = 'txt_todo';
+    var LS_KEY = 'teratail.15262.txt_todo.2';
@@ -226,51 +252,139 @@
     function add_todo() {
+      var value;
+      var item_value;
       /**
+       * 入力欄の入力値を取得
+       */
+      var value = document.getElementById('txt_todo').value;
+      /**
+       * 入力値が空、あるいはスペースのみの場合、項目を追加しない
+       */
+      if (value.trim() !== '') {
+        /**
-       * 入力欄の入力値を取得し、Stringオブジェクトに変換
+         * 入力欄の入力値をStringオブジェクトに変換
+         *
+         * Note: ここでStringオブジェクトに変換しているのは、重複した値を持つ項目を識別するため
+         *       プリミティブ値(普通の文字列)では同じ値の項目を区別できないが、
+         *       オブジェクトは区別できる
+         */
+        item_value = new String(value);
+        /**
+         * ToDoの情報を配列に追加
+         */
+        todo_items.push(item_value);
+        /**
+         * HTML内に要素を追加する
+         */
+        add_item_elem(item_value);
+        /**
+         * データを保存
+         */
+        save();
+      }
+    }
+    /**
+     * ToDoの項目をHTML内に挿入する関数
+     * @param {!String} item_value 挿入する項目の値
+     */
+    function add_item_elem(item_value) {
+      var item_elem;
+      var item_del_button_elem;
+      /**
+       * この処理は、項目に対応する以下のHTMLを生成しています
+       * (厳密には異なります)
        *
-       * Note: ここでStringオブジェクトに変換しているのは、重複した値を持つ項目を識別するため
+       * <li>
-       *       プリミティブ値(普通の文字列)では同じ値の項目を区別できないが、
-       *       オブジェクトは区別できる
+       *   {item_value}
+       *   <input type=button value=del onclick=del_todo>
+       * </li>
        */
+      item_elem = document.createElement('li');
+      item_elem.appendChild(document.createTextNode(item_value + ''));
-      var item_value = new String(document.getElementById('txt_todo').value);
+      item_del_button_elem = document.createElement('input');
+      item_del_button_elem.type = 'button';
+      item_del_button_elem.value = 'del';
+      item_del_button_elem.addEventListener('click', del_todo_create(item_value), false);
+      item_elem.appendChild(item_del_button_elem);
       /**
-       * ToDoの情報を配列に追加
+       * HTMLを挿入
        */
-      todo_items.push(item_value);
+      document.getElementById('view_todo').appendChild(item_elem);
-      /**
-       * HTML内に要素を追加する
-       */
-      add_item_elem(item_value);
-      /**
-       * データを保存
-       */
-      save();
     }
@@ -278,61 +392,67 @@
     /**
-     * ToDoの項目をHTML内に挿入する関数
-     * @param {!String} item_value 挿入する項目の値
-     */
-    function add_item_elem(item_value) {
-      var item_elem;
-      var item_del_button_elem;
-      /**
-       * この処理は、項目に対応する以下のHTMLを生成しています
-       * (厳密には異なります)
-       *
-       * <li>
-       *   {item_value}
-       *   <input type=button value=del onclick=del_todo>
-       * </li>
-       */
-      item_elem = document.createElement('li');
-      item_elem.appendChild(document.createTextNode(item_value + ''));
-      item_del_button_elem = document.createElement('input');
-      item_del_button_elem.type = 'button';
-      item_del_button_elem.value = 'del';
-      item_del_button_elem.addEventListener('click', del_todo_create(item_value), false);
-      item_elem.appendChild(item_del_button_elem);
-      /**
-       * HTMLを挿入
-       */
-      document.getElementById('view_todo').appendChild(item_elem);
+     * 「HTML内のToDoの項目を削除する関数」を生成する関数
+     *
+     * Note: わざわざ「削除する関数を生成する関数」を定義しているのは、
+     *       削除に必要な情報である項目の値(item_value)を
+     *       クロージャで保持するため
+     *
+     * @param {!String} item_value 削除する項目の値
+     * @return {function(Event)} HTML内のToDoの項目を削除する関数
+     */
+    function del_todo_create(item_value) {
+      return function (event) {
+        var item_elem;
+        /**
+         * ToDoの情報を配列から削除
+         */
+        todo_items = todo_items.filter(function (value) {
+          return value !== item_value;
+        });
+        /**
+         * HTML内の項目を削除
+         */
+        item_elem = event.currentTarget.parentNode;
+        item_elem.parentNode.removeChild(item_elem);
+        /**
+         * 保存
+         */
+        save();
+      };
     }
@@ -340,67 +460,103 @@
     /**
-     * 「HTML内のToDoの項目を削除する関数」を生成する関数
-     *
-     * Note: わざわざ「削除する関数を生成する関数」を定義しているのは、
-     *       削除に必要な情報である項目の値(item_value)を
-     *       クロージャで保持するため
-     *
-     * @param {!String} item_value 削除する項目の値
-     * @return {function(Event)} HTML内のToDoの項目を削除する関数
-     */
-    function del_todo_create(item_value) {
-      return function (event) {
-        var item_elem;
-        /**
-         * ToDoの情報を配列から削除
-         */
-        todo_items = todo_items.filter(function (value) {
-          return value !== item_value;
-        });
-        /**
-         * HTML内の項目を削除
-         */
-        item_elem = event.currentTarget.parentNode;
-        item_elem.parentNode.removeChild(item_elem);
-        /**
-         * 保存
-         */
-        save();
-      };
+     * ページが読み込まれたら、localStorageから情報を取り出しHTMLに展開する関数
+     */
+    function load() {
+      var todo_json_data;
+      var todo_saved_data;
+      if (IS_SUPPORT_LS) {
+        /**
+         * localStorageからデータを読み込む
+         */
+        todo_json_data = localStorage.getItem(LS_KEY);
+        /**
+         * localStorageから正しく読み込めたか、
+         * またそのデータが存在するか判定
+         */
+        if (todo_json_data) {
+          try {
+            /**
+             * localStorageから読み込んだJSONをJavaScriptオブジェクトに変換
+             */
+            todo_saved_data = JSON.parse(todo_json_data);
+            /**
+             * 読み込んだデータが配列か、また配列の項目数が1以上か判定
+             */
+            if (Array.isArray(todo_saved_data) && 0 < todo_saved_data.length) {
+              /**
+               * 読み込んだデータに合わせ、HTMLを挿入
+               * また、データ内の文字列をStringオブジェクトに変換
+               *
+               * Note: ここでStringオブジェクトに変換しているのは、重複した値を持つ項目を識別するため
+               *       プリミティブ値(普通の文字列)では同じ値の項目を区別できないが、
+               *       オブジェクトは区別できる
+               */
+              todo_items = todo_saved_data.map(function (v) {
+                // 各項目に相当する文字列を、Stringオブジェクトに変換
+                var value = new String(v);
+                // 項目に対応するHTMLを挿入
+                add_item_elem(value);
+                // Stringオブジェクトを返す
+                return value;
+              });
+            }
+          } catch (e) { }
+        }
+      }
     }
@@ -408,101 +564,21 @@
     /**
-     * ページが読み込まれたら、localStorageから情報を取り出しHTMLに展開する関数
+     * localStorageに情報を保存する関数
-     */
+     */
-    function load() {
+    function save() {
       var todo_json_data;
-      var todo_saved_data;
       if (IS_SUPPORT_LS) {
-        /**
-         * localStorageからデータを読み込む
-         */
-        todo_json_data = localStorage.getItem(LS_KEY);
-        /**
-         * localStorageから正しく読み込めたか、
-         * またそのデータが存在するか判定
-         */
-        if (todo_json_data) {
-          try {
-            /**
-             * localStorageから読み込んだJSONを配列に変換
-             */
-            todo_saved_data = JSON.parse(todo_json_data);
+        todo_json_data = JSON.stringify(todo_items);
-            /**
-             * 読み込んだデータが配列か、また配列の項目数が1以上か判定
-             */
-            if (Array.isArray(todo_saved_data) && 0 < todo_saved_data.length) {
-              /**
-               * 読み込んだデータに合わせ、HTMLを挿入
-               * また、データ内の文字列をStringオブジェクトに変換
-               *
-               * Note: ここでStringオブジェクトに変換しているのは、重複した値を持つ項目を識別するため
-               *       プリミティブ値(普通の文字列)では同じ値の項目を区別できないが、
-               *       オブジェクトは区別できる
-               */
-              todo_items = todo_saved_data.map(function (v) {
+        localStorage.setItem(LS_KEY, todo_json_data);
-                // 各項目に相当する文字列を、Stringオブジェクトに変換
-                var value = new String(v);
-                // 項目に対応するHTMLを挿入
-                add_item_elem(value);
-                // Stringオブジェクトを返す
-                return value;
-              });
-            }
-          } catch (e) { }
-        }
       }
@@ -512,30 +588,6 @@
     /**
-     * localStorageに情報を保存する関数
-     */
-    function save() {
-      var todo_json_data;
-      if (IS_SUPPORT_LS) {
-        todo_json_data = JSON.stringify(todo_items);
-        localStorage.setItem(LS_KEY, todo_json_data);
-      }
-    }
-    /**
      * ページが読み込まれた際にload関数が実行されるよう設定
      * Note: この処理は以下とほぼ等価:
@@ -563,29 +615,3 @@
 </html>
 ```
-改善点として、
-* ToDoは項目リストなのだから、`ul`、`li`要素を利用するべき
-* `onXXX`等の属性を利用してイベントを登録する方法は、イベントを重複して登録できない等の問題からあまり使用するべきではない。
-  (このコードでは重複登録はないが、利用者のアドオン等がそのような実装になっている場合も。その場合、利用者のアドオンが正常に動作しない可能性が考えられる)
-  [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)も併用)を利用したほうが良い。
-* `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)を利用した方が良い。
-* `window.localStorage`オブジェクトは全てのブラウザが対応しているわけではない。`window.localStorage`を利用する処理を行う前に、これに対応しているか検証するべき。
-* 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)に変換し、保存したほうが良い。これにより、利用者のハードディスクの使用量を軽減できる。
-  なお、[JSON](https://ja.wikipedia.org/wiki/JavaScript_Object_Notation)から配列に戻す場合は[JSON.parse](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse)を使用。
-が挙げられます。

冒頭の文章で、伝わりにくいと判断した表現に加筆

2015/09/03 18:51

投稿

sounisi5011

スコア697

test CHANGED Viewed

@@ -4,9 +4,9 @@
-* localStorageに対応するデータが無い場合が想定できていない
+* localStorageに対応するデータが保存されていない(初回利用時の)場合が想定できていない
-* 削除した時に保存されていない
+* 項目を削除した時にデータが保存されていない