javascriptの重複したコードを整えたい

javascriptを習い始め自分で簡単なlocalStorageを使った予定表アプリを作って一応大まかなところはできたのですが、重複したコードがとても多いです。ライブラリなど一切使わずhtmlとjavascriptだけで作りたいのですが、何かコード量を減らす方法を教えてください。

index.html

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>スケジュルアプリ</title>
  <link rel="stylesheet" href="index.css">
  <script src="index.js"></script>
</head>
<body>

  <table>
    <tr>
      <th>朝</th> 
      <td><textarea id="scheduleatmorning" cols="30" rows="3"></textarea></td>
      <td><input type="button" value="保存朝" onclick=savemorning(); ></td>          
    </tr>
    <tr>
      <th>昼</th> 
      <td><textarea id="scheduleatnoon" cols="30" rows="3"></textarea></td>    
      <td><input type="button" value="保存昼" onclick=savenoon(); ></td>      
    </tr>
    <tr>
      <th>夜</th> 
      <td><textarea id="scheduleatnight" cols="30" rows="3"></textarea></td>  
      <td><input type="button" value="保存夜" onclick=savenight(); ></td>        
    </tr>
  </table>
  
</body>
</html>

index.js

'use strict';

var Morning = "";
var Noon = "";
var Night = "";

if (localStorage.getItem('ScheduleAtMorning')) {
  Morning = localStorage.getItem('ScheduleAtMorning');
  document.write(Morning);
}

if (localStorage.getItem('ScheduleAtNoon')) {
  Noon = localStorage.getItem('ScheduleAtNoon');
  document.write(Noon);
}

if (localStorage.getItem('ScheduleAtNight')) {
  Night = localStorage.getItem('ScheduleAtNight');
  document.write(Night);
}

function savemorning() {
  var ScheduleAtMorning = document.getElementById('scheduleatmorning').value;
  localStorage.setItem('ScheduleAtMorning', ScheduleAtMorning);
  reload();
}

function savenoon() {
  var ScheduleAtNoon = document.getElementById('scheduleatnoon').value;
  localStorage.setItem('ScheduleAtNoon', ScheduleAtNoon);
  reload();
}

function savenight() {
  var ScheduleAtNight = document.getElementById('scheduleatnight').value;
  localStorage.setItem('ScheduleAtNight', ScheduleAtNight);
  reload();
}

function reload() {
  window.location.reload();
}
コード

行動規範の内容に同意します

回答3件

ほぼ同等の機能は関数化して再利用できるようにしてください
ただ、document.writeでの処理はやめた方がよいでしょう

投稿2020/06/07 03:46

yambejp

総合スコア116724

takespring

2020/06/07 10:03

ご指摘ありがとうございますm(_ _)m

行動規範の内容に同意します

ベストアンサー

こんにちは。一例を示します。

index.html

body の直後に、 <span id="schedules"></span> を追加
ボタンの onclick を save(朝昼夜を表す文字列) のように修正

html
1<span id="schedules"></span>
2<table>
3  <tr>
4    <th>朝</th>
5    <td><textarea id="scheduleatmorning" cols="30" rows="3"></textarea></td>
6    <td><input type="button" value="保存朝" onclick="save('morning')" ></td>
7  </tr>
8  <tr>
9    <th>昼</th>
10    <td><textarea id="scheduleatnoon" cols="30" rows="3"></textarea></td>
11    <td><input type="button" value="保存昼" onclick="save('noon')" ></td>
12  </tr>
13  <tr>
14    <th>夜</th>
15    <td><textarea id="scheduleatnight" cols="30" rows="3"></textarea></td>
16    <td><input type="button" value="保存夜" onclick="save('night')" ></td>
17  </tr>
18</table>

index.js

reload を、window.location.reload　を使わずに、<span id="schedules"> の内容を書き換えるように修正
3つのボタンのクリックハンドラとして、1つの関数 save を使用
ある文字列の、先頭1文字だけを大文字にするメソッド capitalize を追加（https://stackoverflow.com/a/3291856 から拝借）
初回の画面描画時に、reload が呼ばれるようにする。

javascript
1'use strict';
2
3// ???? https://stackoverflow.com/a/3291856
4String.prototype.capitalize = function() {
5  return this.charAt(0).toUpperCase() + this.slice(1);
6};
7
8function reload() {
9  const schedulesText = ["morning", "noon", "night"].map(name =>
10    localStorage.getItem(`ScheduleAt${name.capitalize()}`)
11  ).join('');
12  document.getElementById('schedules').textContent = schedulesText;
13}
14
15function save(name) {
16  const { value } = document.getElementById(`scheduleat${name}`);
17  const key = `ScheduleAt${name.capitalize()}`;
18  localStorage.setItem(key, value);
19  reload();
20}
21
22document.addEventListener('DOMContentLoaded', reload);
23

動作確認用CodePen: https://codepen.io/jun68ykt/pen/NWxqyBB?editors=1010

追記

さらに、index.html のほうでは、<table> の行を書かないでおいて、index.js で行を追加することも考えられます。このようにすると、朝、昼、夜のほかに、新しい時間帯を追加したいときにも、修正箇所を最小限にすることができます。以下、この趣旨による修正版です。

index.html

html
1<span id="schedules"></span>
2<table>
3</table>

index.js

javascript
1'use strict';
2
3// ???? https://stackoverflow.com/a/3291856
4String.prototype.capitalize = function() {
5  return this.charAt(0).toUpperCase() + this.slice(1);
6};
7
8const PERIODS_OF_A_DAY = [
9  { name: "morning", label: "朝" },
10  { name: "noon", label: "昼" },
11  { name: "night", label: "夜" }
12];
13
14function reload() {
15  const schedulesText = PERIODS_OF_A_DAY.map(({ name }) =>
16    localStorage.getItem(`ScheduleAt${name.capitalize()}`)
17  ).join('');
18  document.getElementById('schedules').textContent = schedulesText;
19}
20
21function save(name) {
22  const { value } = document.getElementById(`scheduleat${name}`);
23  const key = `ScheduleAt${name.capitalize()}`;
24  localStorage.setItem(key, value);
25  reload();
26}
27
28const tableRow = ({name, label}) =>
29  `<tr>
30     <th>${label}</th>
31     <td><textarea id="scheduleat${name}" cols="30" rows="3"></textarea></td>
32     <td><input type="button" value="保存${label}" onclick="save('${name}')" ></td>
33   </tr>`;
34
35document.addEventListener('DOMContentLoaded', function() {
36  const table = document.querySelector('table');
37  table.innerHTML = PERIODS_OF_A_DAY.map(tableRow).join('');
38  reload();
39});
40

上記のコードにある、以下の定数 PERIODS_OF_A_DAY

javascript
1const PERIODS_OF_A_DAY = [
2  { name: "morning", label: "朝" },
3  { name: "noon", label: "昼" },
4  { name: "night", label: "夜" }
5];

に、たとえば以下のように、夕方と深夜を追加すると、これらに対応した行が追加されます。

javascript
1const PERIODS_OF_A_DAY = [
2  { name: "morning", label: "朝" },
3  { name: "noon", label: "昼" },
4  { name: "evening", label: "夕方" },
5  { name: "night", label: "夜" },
6  { name: "midnight", label: "深夜" },
7];

動作確認用CodePen: https://codepen.io/jun68ykt/pen/XWXbEXX?editors=1010

投稿2020/06/07 04:55

編集2020/06/07 05:56

jun68ykt

総合スコア9058

takespring

2020/06/07 10:01

ありがとうございますm(_ _)m 本当に参考になりました。参考にさせていただきます。

jun68ykt

2020/06/07 11:12

どういたしまして???? > 本当に参考になりました。とのことでよかったです。

takespring

2020/06/08 03:24 編集

すみません。二点質問させていただきます。どうして.join('')と付け足して空の文字列を結合させているのか、なぜdocument.getElementById(`scheduleat${name}`)と.valueを付け足さなくてもtextarea内の値を取得できるかをお聞きしたいです。お手数おかけしますがよろしくお願いします。

jun68ykt

2020/06/09 07:35 編集

こんにちは (1)どうして.join('')と付け足して空の文字列を結合させているのか【回答】もし、.join('')を付け足さないと、 table.innerHTML = PERIODS_OF_A_DAY.map(tableRow); となり、上記の右辺は文字列の配列になりますが、innerHTML　には文字列を入れなければならないです。そのために連結した文字列を作るために .join('') を足しています。さらにこの join の引数の "" がないと、連結の区切りにカンマが追加されてしまいますので、この区切り文字に何も使わないようにするために空文字列の "" を指定しています。 (2) document.getElementById(`scheduleat${name}`)と.valueを付け足さなくてもtextarea内の値を取得できるか【回答】 const { value } = document.getElementById(`scheduleat${name}`); のことだと思いますが、これはオブジェクトの分割代入を使っています。詳しくは、以下のページ https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment の中の「オブジェクトの分割代入」の説明を参照してください。簡単にいうと、 e というオブジェクトがあり、これの value プロパティを変数 value として取り出したいときに const value = e.value; と value を２個書かなくても、 const { value } = e; とすれば済む、というものです。

takespring

2020/06/10 06:38 編集

理解しました。知らないことを知れてよかったです。ありがとうございましたm(_ _)m

行動規範の内容に同意します

コード量を減らした例を以下に示します。
詳しい解説は省略するので元のコードと見比べてみてください。

html
1<!-- tableタグ内のみ抜粋 -->
2  <table>
3    <tr>
4      <th>朝</th> 
5      <td><textarea id="scheduleatmorning" cols="30" rows="3"></textarea></td>
6      <!-- onclickの中身を変更 -->
7      <td><input type="button" value="保存朝" onclick="save('ScheduleAtMorning');" ></td>          
8    </tr>
9    <tr>
10      <th>昼</th> 
11      <td><textarea id="scheduleatnoon" cols="30" rows="3"></textarea></td>    
12      <!-- onclickの中身を変更 -->
13      <td><input type="button" value="保存昼" onclick="save('ScheduleAtNoon');" ></td>      
14    </tr>
15    <tr>
16      <th>夜</th> 
17      <td><textarea id="scheduleatnight" cols="30" rows="3"></textarea></td>  
18      <!-- onclickの中身を変更 -->
19      <td><input type="button" value="保存夜" onclick="save('ScheduleAtNight');" ></td>        
20    </tr>
21  </table>

js
1'use strict';
2
3var keys = ['ScheduleAtMorning', 'ScheduleAtNoon', 'ScheduleAtNight'];
4
5for (var i = 0; i < keys.length; i++) {
6  var value = localStorage.getItem(keys[i]);
7  if (value) {
8    document.write(value);
9  }
10}
11
12function save(key) {
13  var value = document.getElementById(key.toLowerCase()).value;
14  localStorage.setItem(key, value);
15  window.location.reload();
16}