回答編集履歴

2 文章表現を修正

sounisi5011

sounisi5011 score 703

2016/07/03 16:10  投稿

```JavaScript
var count1 = 0;
counter();
function counter(){
 count1 += 1;
 save();
}
window.onload = function(){
   var count_load = localStorage.getItem("count1");
   document.getElementById("dayCount").innerHTML = count_load;
}
function save(){
   localStorage.setItem("count1" , document.getElementById("dayCount").innerHTML);
}
```
このコードを見る限りでは、以下の問題が挙げられます。
* 変数`count1`が、初期化とカウントアップ以外で使用されていない。
---
ページを読み込んだ後、このコードは以下の順に動作します。
0. `var count1 = 0;`を実行。
   変数`count1`の値に`0`を代入して初期化。
0. `counter();`を実行。
   `counter`関数を呼び出す。
0. `count1 += 1;`を実行。
   変数`count1`の値をカウントアップし、`1`に変更。
0. `save();`を実行。
   `save`関数を呼び出す。
0. `localStorage.setItem("count1" , document.getElementById("dayCount").innerHTML);`を実行。
   `<div id="dayCount"></div>`内の`innerHTML`の値(この時点では空文字列となる値)を、LocalStorageに`count1`キーと関連付けて保存。
0. `window.onload`に関数を設定。
0. ページ読み込みが完了し、`window.onload`に設定した関数を実行。
0. ページ読み込みが完了し、`window.onload`に設定した関数が実行される。
0. `var count_load = localStorage.getItem("count1");`を実行。
   LocalStorageに`count1`キーと関連付けて保存した値(空文字列)を取り出し、変数`count_load`に代入。
0. `document.getElementById("dayCount").innerHTML = count_load;`を実行。
   `<div id="dayCount"></div>`内の`innerHTML`の値を、変数`count_load`の値(空文字列)に変更。
このうち、カウントに使用されている(と思われる)変数`count1`は、1番目と3番目の処理、
コードで言えば、以下の箇所のみで使用されているように見えます。
```JavaScript
var count1 = 0;
```
```JavaScript
 count1 += 1;
```
値を`0`に初期化した後、カウントアップして`1`にしている。ただそれだけです。
「この変数の値がLocalStorageに保存される」事がこのコードの要件(本来求める動作)だと考えられますが、このコードでLocalStorageに保存されているのは、id属性に`dayCount`が指定された要素(`<div id="dayCount"></div>`)内の「`innerHTML`の値」だけです。
そしてこの「`innerHTML`の値」の空文字列をLocalStorageに保存してから、再度取り出し、
再び`innerHTML`へと設定しなおしています。
が、この間にこの空文字列がカウントの値へ変更されてはいません。
これが、カウントアップされない原因です。そもそもカウントの値が保存されていません。
実際、以下の実際に書いてみたサンプルでもカウントアップどころか、何も表示されない状態になっています。
※空文字列は0文字の文字列データのため、何も表示されない。
[LocalStorageを利用したカウントアップのサンプルコード](http://output.jsbin.com/juhure)
---
解決するには、変数`count1`の値をLocalStorageに保存するようにしましょう。
まず、変数`count1`を初期化するときに、`0`をそのまま代入するのではなく、LocalStorageから読みだした値を変数`count1`に代入するようにします。
ただし、LocalStorageにカウントの値が無い場合には、代わりに`0`を代入するようにします。
このあたりの処理は、「LocalStorageからカウントの値を読み出し、カウントの数値か`0`を返す」関数としてひとまとめにしてしまったほうが良いと思います。
ここでは、これらの処理を`load`関数にまとめることにします。
`load`関数で取得したカウントの値を変数`count1`に代入し、それから変数`count1`をカウントアップします。
そして、カウントアップした値をLocalStorageに保存して、`innerHTML`の値も更新します。
この時、`innerHTML`の値をLocalStorageに保存するような処理にしては駄目です。
LocalStorageに保存するのは、変数`count1`の値です。
そして`innerHTML`は、変数`count1`の値を**書き込むだけ**にしましょう。
以上の処理を、**全て「`window.onload`に設定する関数」の中で書きます**。
「`window.onload`に指定された関数」は非同期処理されるため、外側のコードが最後まで実行された後に呼び出されてしまいます。
このため、外側と内側とでコードの実行タイミングが異なるようになってしまいます。
これを混同するとえらく面倒になるうえに、殆どの場合は期待した動作をしてくれなくなるので、
全ての処理は「`window.onload`に設定する関数」の中に書いてしまいましょう。
ただし、関数定義は除きます。
関数定義はあくまで関数を定義するだけであって、その場で動作するコードではないため、`window.onload`の外に書いても問題はありません。
※関数定義を内側に書いても間違いではありません。動作します。このあたりは、見やすさや好みの差だと私は思います。この例では見やすさのため、外側に書くことにします。
以上の修正案を具体的なコードにしたものが以下になります。
```JavaScript
window.onload = function() {
 /**
  * @type {number}
  *
  * LocalStorageから、カウントの値を読み込む。
  */
 var count1 = load();
 /* カウントアップ */
 count1 += 1;
 /* カウントアップした値を保存 */
 save(count1);
 /* カウントを表示 */
 document.getElementById("dayCount").innerHTML = count1;
};
/**
* LocalStorageに保存しておいたカウントの値を数値で取り出す。
*
* LocalStorageに値が保存されていない場合、代わりに0を返す。
* また、値が正の整数以外の場合も0を返す。
* (LocalStorageの値は簡単に改変されてしまうため、その対策)
*
* @return {number} カウントを示す数値
*/
function load() {
 /**
  * @type {string|null}
  *
  * LocalStorageに保存されていた文字列値を取り出す。
  * LocalStorageに値が保存されていない場合、代わりにnullが返される。
  */
 var count_data = localStorage.getItem("count1");
 /**
  * @type {number}
  *
  * 保存されていた値を数値に変換。nullの場合は0になる。
  * また、数値に変換できない文字列値はNaN(非数)という特殊な数値になる。
  */
 var count = Number(count_data);
 /*
  * 値が適切な数値(0とNaN以外の数値であり、かつ、1以上の数値であり、かつ、整数値である)かどうか判定。
  * 条件に合致した場合は数値をそのまま返し、それ以外の場合は0を返す。
  */
 if (
   count &&                   // 0とNaN以外の数値か判定
   1 <= count &&              // 1以上の数値か判定
   Math.floor(count) === count // 整数値か判定
 ) {
   return count;
 } else {
   return 0;
 }
}
/**
* LocalStorageにカウントの値を保存する
*
* @param {number} count LocalStorageに保存するカウントの値
*/
function save(count) {
 localStorage.setItem("count1", count);
}
```
動作サンプル:[LocalStorageを利用したカウントアップのサンプルコード](http://output.jsbin.com/siradi)
1 改めて読みなおしたところ、分かりにくいと感じた表現や文章を修正

sounisi5011

sounisi5011 score 703

2016/07/03 13:23  投稿

```JavaScript
var count1 = 0;
counter();
function counter(){
 count1 += 1;
 save();
}
window.onload = function(){
   var count_load = localStorage.getItem("count1");
   document.getElementById("dayCount").innerHTML = count_load;
}
function save(){
   localStorage.setItem("count1" , document.getElementById("dayCount").innerHTML);
}
```
このコードを見る限りでは、以下の問題が挙げられます。
* 変数`count1`が、初期化とカウントアップ以外で使用されていない。
---
ページを読み込んだ後、このコードは以下の順に動作します。
0. `var count1 = 0;`を実行。
   変数`count1`の値に`0`を代入して初期化。
0. `counter();`を実行。
   `counter`関数を呼び出す。
0. `count1 += 1;`を実行。
   変数`count1`の値をカウントアップし、`1`に変更。
0. `save();`を実行。
   `save`関数を呼び出す。
0. `localStorage.setItem("count1" , document.getElementById("dayCount").innerHTML);`を実行。
   `<div id="dayCount"></div>`内の`innerHTML`の値(この時点では空文字列となる値)を、LocalStorageに`count1`キーと関連付けて保存。
0. `window.onload`に関数を設定。
0. ページ読み込みが完了し、`window.onload`に設定した関数を実行。
0. `var count_load = localStorage.getItem("count1");`を実行。
   LocalStorageに`count1`キーと関連付けて保存した値(空文字列)を取り出し、変数`count_load`に代入。
0. `document.getElementById("dayCount").innerHTML = count_load;`を実行。
   `<div id="dayCount"></div>`内の`innerHTML`の値を、変数`count_load`の値(空文字列)に変更。
このうち、カウントに使用されている変数`count1`は、1番目と3番目の処理、
このうち、カウントに使用されている(と思われる)変数`count1`は、1番目と3番目の処理、
コードで言えば、以下の箇所のみで使用されているように見えます。
```JavaScript
var count1 = 0;
```
```JavaScript
 count1 += 1;
```
値を`0`に初期化した後、カウントアップして`1`にしている。ただそれだけです。
「この変数の値がLocalStorageに保存される」事がこのコードの要件だと考えられますが、このコードでLocalStorageに保存されているのは、id属性に`dayCount`が指定された要素(`<div id="dayCount"></div>`)内の「`innerHTML`の値」だけです。
「この変数の値がLocalStorageに保存される」事がこのコードの要件(本来求める動作)だと考えられますが、このコードでLocalStorageに保存されているのは、id属性に`dayCount`が指定された要素(`<div id="dayCount"></div>`)内の「`innerHTML`の値」だけです。
そしてこの「`innerHTML`の値」の空文字列をLocalStorageに保存してから、再度取り出し、
再び`innerHTML`へと設定しなおしています。
が、この間にこの空文字列がカウントの値へ変更されてはいません。
これが、カウントアップされない原因です。そもそもカウントの値が保存されていません。
実際、以下の書いてみたサンプルでもカウントアップどころか、何も表示されない状態になっています。
実際、以下の実際に書いてみたサンプルでもカウントアップどころか、何も表示されない状態になっています。
※空文字列は0文字の文字列データのため、何も表示されない。
[LocalStorageを利用したカウントアップのサンプルコード](http://output.jsbin.com/juhure)
---
解決するには、変数`count1`の値をLocalStorageに保存するようにしましょう。
まず、変数`count1`を初期化するときに、`0`をそのまま代入するのではなく、LocalStorageから読みだした値を変数`count1`に代入するようにします。
ただし、LocalStorageにカウントの値が無い場合には、代わりに`0`を代入するようにします。
このあたりの処理は、「LocalStorageからカウントの値を読み出し、カウントの数値か、`0`を返す」関数としてひとまとめにしてしまったほうが良いと思います。
このあたりの処理は、「LocalStorageからカウントの値を読み出し、カウントの数値か`0`を返す」関数としてひとまとめにしてしまったほうが良いと思います。
ここでは、これらの処理を`load`関数にまとめることにします。
`load`関数でカウントの値を代入した変数`count1`をカウントアップし、
それをLocalStorageに保存して、`innerHTML`の値も更新します。
`load`関数で取得したカウントの値を変数`count1`に代入し、それから変数`count1`をカウントアップします。
そして、カウントアップした値をLocalStorageに保存して、`innerHTML`の値も更新します。
この時、`innerHTML`の値をLocalStorageに保存するような処理にしては駄目です。
LocalStorageに保存するのは、変数`count1`の値です。
そして`innerHTML`は、変数`count1`の値を**書き込むだけ**にしましょう。
以上の処理を、**全て「`window.onload`に設定する関数」の中で書きます**。
「`window.onload`に指定された関数」は非同期処理されるため、外側のコードが最後まで実行された後に呼び出されてしまいます。
このため、外側と内側とでコードの実行タイミングが異なるようになってしまいます。
これを混同するとえらく面倒になるうえに、殆どの場合は期待した動作をしてくれなくなるので、全ての処理は「`window.onload`に設定する関数」の中に書いてしまいましょう。
これを混同するとえらく面倒になるうえに、殆どの場合は期待した動作をしてくれなくなるので、
全ての処理は「`window.onload`に設定する関数」の中に書いてしまいましょう。
ただし、関数定義は除きます。
関数定義はあくまで関数を定義するだけであって、その場で動作するコードではないため、`window.onload`の外に書いても問題はありません。
※関数定義を内側に書いても間違いではありません。動作します。このあたりは、見やすさや好みの差だと私は思います。この例では見やすさのため、外側に書くことにします。
以上の修正案を具体的なコードにしたものが以下になります。
```JavaScript
window.onload = function() {
 /**
  * @type {number}
  *
  * LocalStorageから、カウントの値を読み込む。
  */
 var count1 = load();
 /* カウントアップ */
 count1 += 1;
 /* カウントアップした値を保存 */
 save(count1);
 /* カウントを表示 */
 document.getElementById("dayCount").innerHTML = count1;
};
/**
* LocalStorageに保存しておいたカウントの値を数値で取り出す。
*
* LocalStorageに値が保存されていない場合、代わりに0を返す。
* また、値が正の整数以外の場合も0を返す。
* (LocalStorageの値は簡単に改変されてしまうため、その対策)
*
* @return {number} カウントを示す数値
*/
function load() {
 /**
  * @type {string|null}
  *
  * LocalStorageに保存されていた文字列値を取り出す。
  * LocalStorageに値が保存されていない場合、代わりにnullが返される。
  */
 var count_data = localStorage.getItem("count1");
 /**
  * @type {number}
  *
  * 保存されていた値を数値に変換。nullの場合は0になる。
  * また、数値に変換できない文字列値はNaN(非数)という特殊な数値になる。
  */
 var count = Number(count_data);
 /*
  * 値が適切な数値(0とNaN以外の数値であり、かつ、1以上の数値であり、かつ、整数値である)かどうか判定。
  * 条件に合致した場合は数値をそのまま返し、それ以外の場合は0を返す。
  */
 if (
   count &&                   // 0とNaN以外の数値か判定
   1 <= count &&              // 1以上の数値か判定
   Math.floor(count) === count // 整数値か判定
 ) {
   return count;
 } else {
   return 0;
 }
}
/**
* LocalStorageにカウントの値を保存する
*
* @param {number} count LocalStorageに保存するカウントの値
*/
function save(count) {
 localStorage.setItem("count1", count);
}
```
動作サンプル:[LocalStorageを利用したカウントアップのサンプルコード](http://output.jsbin.com/siradi)

思考するエンジニアのためのQ&Aサイト「teratail」について詳しく知る