javascriptコードへのご意見頂きたいです

Question

### 前提こんにちは。いつもお世話になっております。現在web開発の勉強をしている学生です。簡単なwebページの作成を行っているのですが、自分の書いたコードをなかなかコンパクトにできず気持ち悪いです。可能な限り冗長なコードをなくしたつもりですが、まだ改善できる点などありますでしょうか。厳しい意見もためになりますので、よろしくお願いいたします。 ### 実現したいこと冗長なコードの削除 ### 発生している問題・エラーメッセージ ``` 特になし ``` ### 該当のソースコード ```JavaScript window.addEventListener('DOMContentLoaded', function(){ const Qreform = document.getElementById('Qreform'); Qreform.addEventListener('input', function(event){ if (event.target.name == "country") { // プルダウンの処理 var show1 = document.getElementById('disp1'); show1.innerText = event.target.value; } else if (event.target.name == "schedule") { // ラジオボタンの処理 var show2 = document.getElementById('disp2'); show2.innerText = event.target.value; } else if (event.target.name == "place") { // チェックボックスの処理 let checked = document.querySelectorAll('input[name="place"]:checked'); let out = []; checked.forEach((chk) => { out.push(chk.value); }); var show3 = document.getElementById('disp3'); show3.innerText = out; } else if (event.target.name == "budget") { let text = document.querySelectorAll('input[name="budget"]'); let out = []; text.forEach((txt) => { out.push(txt.value); }); var show4 = document.getElementById('disp4'); show4.innerText = out + "円"; } else{ } }); }); ``` ```html questionnaire

研修先希望調査
派遣先
日程	3月 8月 12月
宿泊施設	A-HOTEL B-HOTEL C-RESORT
予算	円

``` ### 試したこと以前の質問で教えて頂いたこと(ループ処理)などを実装。 ### 補足情報（FW/ツールのバージョンなど） HTML5 Atom

Accepted Answer

はじめに質問の本題に対する修正の一例を回答します。その後、本題以外の部分で修正すべき点を補足します。

HTMLの表示用の要素の id 属性を以下のように修正します。

diff
1-      <td id="disp1"></td>
2+      <td id="disp-country"></td>

diff
1-      <td id="disp2"></td>
2+      <td id="disp-schedule"></td>

diff
1-      <td id="disp3"></td>
2+      <td id="disp-place"></td>

diff
1-      <td id="disp4"></td>
2+      <td id="disp-budget"></td>

このように修正しておくと、JavaScript のほうは下記でよいのではと思います。

javascript
1window.addEventListener('DOMContentLoaded', function () {
2  const Qreform = document.getElementById('Qreform');
3
4  Qreform.addEventListener('input', function ({ target: { value, name } }) {
5    document.getElementById(`disp-${name}`).innerText =
6      name === 'place'
7        ? [...document.querySelectorAll('input[name="place"]:checked')].map(e => e.value)
8        : value;
9  });
10});

補足1

他にHTMLのほうで修正すべき点として下記の3点を提案します。

name="schedule" のラジオボタンの id がどれも r になっており重複しているので、重複しないように要修正
country のオプションが <option value="台湾" />台湾 という書き方になっているが、<option value="台湾">台湾</option> に要修正
country のオプションが初期表示のときに、国がどこも選ばれていないことを示すために <option value=""></option> というオプションを一番上に追加しておくとよい

補足2

上記で挙げたコード例に対してのさらなる修正です。

(1) name 属性が place の複数のチェックボックスからチェックされているものの値の配列を得る
[...document.querySelectorAll('input[name="place"]:checked')].map(e => e.value)
は name が他のチェックボックスでも使えそうなので関数化しておきます。

javascript
1const checkedValues = name =>
2  [...document.querySelectorAll(`input[name="${name}"]:checked`)].map(e => e.value);

(2) 表示用要素に入力値をテキスト設定するところを関数化します。このとき、name属性によっては表示用に加工します。

javascript
1const displayValue = (name, value) => {
2  value = name === 'budget' ? `${(+value || 0).toLocaleString()} 円` : value;
3  document.getElementById(`disp-${name}`).innerText = value;
4}

上記の(1)(2)を反映したCodepen 👉 https://codepen.io/su507/pen/OJZddEj?editors=1010

Answer

```javascript

研修先希望調査
派遣先
日程	3月 8月 12月
宿泊施設	A-HOTEL B-HOTEL C-RESORT
予算	円

```

Answer

同じようなコードを何度も書くのは嫌だな、と思うのは良い意識だと思います。何度も書くとミスが入りやすいですしね。

この場合は、`name` プロパティ値から表示先のIDと加工関数への表を持っておくとすっきりするかもしれません。
```js
const controlsDisplayMap = {
  place: {
    displayId: 'disp3',
    filter: () => [...document.querySelectorAll('input[name="place"]')].map(e => e.value)
  }
  budget: {
    displayId: 'disp4',
    filter: target => `${target.value}円`
  },
  // country, schedule のぶんも作っておく
};

const entry = controlsDisplayMap[event.target.name];
if (entry) {
  document.qeurySelecotr(`#${entry.displayId}`).textContent =
      entry.filter(event.target);
}
```

前提

実現したいこと

発生している問題・エラーメッセージ

該当のソースコード

試したこと

補足情報（FW/ツールのバージョンなど）

補足1

補足2

関連した質問