質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

新規登録して質問してみよう
ただいま回答率
86.12%
JavaScript

JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。

HTML

HTMLとは、ウェブ上の文書を記述・作成するためのマークアップ言語のことです。文章の中に記述することで、文書の論理構造などを設定することができます。ハイパーリンクを設定できるハイパーテキストであり、画像・リスト・表などのデータファイルをリンクする情報に結びつけて情報を整理します。現在あるネットワーク上のほとんどのウェブページはHTMLで作成されています。

CSS

CSSはXMLやHTMLで表現した色・レイアウト・フォントなどの要素を指示する仕様の1つです。

解決済

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

Keisuke_NUT
Keisuke_NUT

総合スコア3

JavaScript

JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。

HTML

HTMLとは、ウェブ上の文書を記述・作成するためのマークアップ言語のことです。文章の中に記述することで、文書の論理構造などを設定することができます。ハイパーリンクを設定できるハイパーテキストであり、画像・リスト・表などのデータファイルをリンクする情報に結びつけて情報を整理します。現在あるネットワーク上のほとんどのウェブページはHTMLで作成されています。

CSS

CSSはXMLやHTMLで表現した色・レイアウト・フォントなどの要素を指示する仕様の1つです。

3回答

0グッド

1クリップ

886閲覧

投稿2022/10/14 02:01

編集2022/10/14 02:10

前提

こんにちは。いつもお世話になっております。
現在web開発の勉強をしている学生です。
簡単なwebページの作成を行っているのですが、自分の書いたコードをなかなかコンパクトにできず気持ち悪いです。
可能な限り冗長なコードをなくしたつもりですが、まだ改善できる点などありますでしょうか。
厳しい意見もためになりますので、よろしくお願いいたします。

実現したいこと

冗長なコードの削除

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

特になし

該当のソースコード

JavaScript

1window.addEventListener('DOMContentLoaded', function(){ 2 const Qreform = document.getElementById('Qreform'); 3 Qreform.addEventListener('input', function(event){ 4 if (event.target.name == "country") { 5 // プルダウンの処理 6 var show1 = document.getElementById('disp1'); 7 show1.innerText = event.target.value; 8 } else if (event.target.name == "schedule") { 9 // ラジオボタンの処理 10 var show2 = document.getElementById('disp2'); 11 show2.innerText = event.target.value; 12 } else if (event.target.name == "place") { 13 // チェックボックスの処理 14 let checked = document.querySelectorAll('input[name="place"]:checked'); 15 let out = []; 16 checked.forEach((chk) => { 17 out.push(chk.value); 18 }); 19 var show3 = document.getElementById('disp3'); 20 show3.innerText = out; 21 } 22 else if (event.target.name == "budget") { 23 let text = document.querySelectorAll('input[name="budget"]'); 24 let out = []; 25 text.forEach((txt) => { 26 out.push(txt.value); 27 }); 28 var show4 = document.getElementById('disp4'); 29 show4.innerText = out + "円"; 30 } 31 else{ 32 } 33 }); 34}); 35

html

1<!DOCTYPE html> 2<html lang="ja"> 3<head> 4 <title>questionnaire</title> 5 <link rel="stylesheet" type="text/css" href="22331787_Q1.css" /> 6 <script type="text/javascript" src="./22331787.js"></script> 7</head> 8<body> 9 <form id="Qreform"> 10 <table> 11 <caption>研修先希望調査 </caption> 12 <tr> 13 <th> 派遣先</th> 14 <td> 15 <select name="country" > 16 <option value="台湾" />台湾 17 <option value="シンガポール" />シンガポール 18 <option value="フィリピン" />フィリピン 19 </select> 20 </td> 21 <td id="disp1"></td> 22 </tr> 23 <tr> 24 <th>日程</th> 25 <td> 26 <label> 27 <input type="radio" name="schedule" value="3月" id="r"/>3月 28 </label> 29 <label> 30 <input type="radio" name="schedule" value="8月" id="r"/>8月 31 </label> 32 <label> 33 <input type="radio" name="schedule" value="12月" id="r"/>12月 34 </label> 35 </td> 36 <td id="disp2"></td> 37 </tr> 38 <tr> 39 <th>宿泊施設</th> 40 <td> 41 <input type="checkbox" name="place" value="A-HOTEL" id="c1"/> 42 <label for="c1" class="l1">A-HOTEL</label> 43 <input type="checkbox" name="place" value="B-HOTEL" id="c2"/> 44 <label for="c2" class="l1">B-HOTEL</label> 45 <input type="checkbox" name="place" value="C-RESORT" id="c3"/> 46 <label for="c3" class="l1">C-RESORT</label> 47 </td> 48 <td id="disp3"></td> 49 </tr> 50 <tr> 51 <th>予算</th> 52 <td> 53 <input type="text" name="budget">54 </td> 55 <td id="disp4"></td> 56 </tr> 57 <tr> 58 <td colspan="3" id="tail"> 59 <input type="submit" name="sendbutton" value="送信" id="button" /> 60 </td> 61 </tr> 62 </table> 63 </form> 64 65</body> 66</html> 67

試したこと

以前の質問で教えて頂いたこと(ループ処理)などを実装。

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

HTML5
Atom

以下のような質問にはグッドを送りましょう

  • 質問内容が明確
  • 自分も答えを知りたい
  • 質問者以外のユーザにも役立つ

グッドが多くついた質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

気になる質問をクリップする

クリップした質問は、後からいつでもマイページで確認できます。

またクリップした質問に回答があった際、通知やメールを受け取ることができます。

下記のような質問は推奨されていません。

  • 間違っている
  • 質問になっていない投稿
  • スパムや攻撃的な表現を用いた投稿

適切な質問に修正を依頼しましょう。

m.ts10806

2022/10/14 02:05

JavaScriptだけではリファクタリングにならないと思います。 HTMLも提示してください。 また、HTMLもある程度改修する前提でも問題ないのでしょうか。 仕様も提示されたほうが良いと思います。 コードの見た目だけ直しても同じ動作をしなければリファクタリングになりません。
Keisuke_NUT

2022/10/14 02:10

m.ts10806様 いつもコメント・回答ありがとうございます。 感謝しております。 大変失礼しました、HTMLのソースコードも追加しました。 はい、HTMLの改修も問題ございません。 何か気づかれた点や改善点等あれば、ご指摘よろしくお願いいたします。
forer

2022/10/14 02:44

>自分の書いたコードをなかなかコンパクトにできず気持ち悪いです。 どの辺が気持ち悪いのでしょうか?
forer

2022/10/14 02:48

配列にぶち込んでforで回しifの条件式の判定回数を減らす程度ですか? そんなの部外者から見ればどうでもいい話だとおもいますが?
forer

2022/10/14 02:49

コードを見られて馬鹿にされるなどのちんけなプライドは捨てましょう なりふりかまわないほうが、うまくいきますよ

回答3

9

javascript

1<script> 2document.addEventListener('input',e=>{ 3 const f=e.target.closest('#Qreform'); 4 if(!f) return false; 5 disp1.textContent= f.querySelector('[name=country]').value; 6 disp2.textContent= f.querySelector('[name=schedule]:checked')?.value; 7 disp3.textContent= [...f.querySelectorAll('[name=place]:checked')].map(x=>x.value).join(','); 8 const v=f.querySelector('[name=budget]').value; 9 disp4.textContent= v+(/^\d+$/.test(v)?"円":""); 10}); 11</script> 12 <form id="Qreform"> 13 <table> 14 <caption>研修先希望調査 </caption> 15 <tr> 16 <th> 派遣先</th> 17 <td> 18 <select name="country" > 19 <option value="">-</option> 20 <option value="台湾">台湾</option> 21 <option value="シンガポール">シンガポール</option> 22 <option value="フィリピン">フィリピン</option> 23 </select> 24 </td> 25 <td id="disp1"></td> 26 </tr> 27 <tr> 28 <th>日程</th> 29 <td> 30 <label> 31 <input type="radio" name="schedule" value="3月">332 </label> 33 <label> 34 <input type="radio" name="schedule" value="8月">835 </label> 36 <label> 37 <input type="radio" name="schedule" value="12月">1238 </label> 39 </td> 40 <td id="disp2"></td> 41 </tr> 42 <tr> 43 <th>宿泊施設</th> 44 <td> 45 <label> 46 <input type="checkbox" name="place" value="A-HOTEL">A-HOTEL 47 </label> 48 <label> 49 <input type="checkbox" name="place" value="B-HOTEL">B-HOTEL 50 </label> 51 <label> 52 <input type="checkbox" name="place" value="C-RESORT">C-RESORT 53 </label> 54 </td> 55 <td id="disp3"></td> 56 </tr> 57 <tr> 58 <th>予算</th> 59 <td> 60 <input type="text" name="budget">61 </td> 62 <td id="disp4"></td> 63 </tr> 64 <tr> 65 <td colspan="3" id="tail"> 66 <input type="submit" value="送信"> 67 </td> 68 </tr> 69 </table> 70 </form>

投稿2022/10/14 03:27

編集2022/10/14 03:42
yambejp

総合スコア108937

spoofy_dragon😄を押しています
akbr, spoofy_dragon, m.ts10806👍を押しています
spoofy_dragon❤️を押しています
spoofy_dragon😍を押しています
spoofy_dragon👏を押しています
spoofy_dragon🎉を押しています
spoofy_dragonを押しています

良いと思った回答にはグッドを送りましょう。
グッドが多くついた回答ほどページの上位に表示されるので、他の人が素晴らしい回答を見つけやすくなります。

下記のような回答は推奨されていません。

  • 間違っている回答
  • 質問の回答になっていない投稿
  • スパムや攻撃的な表現を用いた投稿

このような回答には修正を依頼しましょう。

4

ベストアンサー

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

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}

投稿2022/10/14 03:37

編集2022/10/14 04:55
退会済みユーザー

退会済みユーザー

総合スコア0

akbr, ka2obushi, Cocode, m.ts10806👍を押しています

良いと思った回答にはグッドを送りましょう。
グッドが多くついた回答ほどページの上位に表示されるので、他の人が素晴らしい回答を見つけやすくなります。

下記のような回答は推奨されていません。

  • 間違っている回答
  • 質問の回答になっていない投稿
  • スパムや攻撃的な表現を用いた投稿

このような回答には修正を依頼しましょう。

2

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

この場合は、name プロパティ値から表示先のIDと加工関数への表を持っておくとすっきりするかもしれません。

js

1const controlsDisplayMap = { 2 place: { 3 displayId: 'disp3', 4 filter: () => [...document.querySelectorAll('input[name="place"]')].map(e => e.value) 5 } 6 budget: { 7 displayId: 'disp4', 8 filter: target => `${target.value}` 9 }, 10 // country, schedule のぶんも作っておく 11}; 12 13const entry = controlsDisplayMap[event.target.name]; 14if (entry) { 15 document.qeurySelecotr(`#${entry.displayId}`).textContent = 16 entry.filter(event.target); 17}

投稿2022/10/14 03:21

編集2022/10/14 03:24
int32_t

総合スコア17087

akbr, m.ts10806👍を押しています

良いと思った回答にはグッドを送りましょう。
グッドが多くついた回答ほどページの上位に表示されるので、他の人が素晴らしい回答を見つけやすくなります。

下記のような回答は推奨されていません。

  • 間違っている回答
  • 質問の回答になっていない投稿
  • スパムや攻撃的な表現を用いた投稿

このような回答には修正を依頼しましょう。

15分調べてもわからないことは
teratailで質問しよう!

ただいまの回答率
86.12%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問

同じタグがついた質問を見る

JavaScript

JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。

HTML

HTMLとは、ウェブ上の文書を記述・作成するためのマークアップ言語のことです。文章の中に記述することで、文書の論理構造などを設定することができます。ハイパーリンクを設定できるハイパーテキストであり、画像・リスト・表などのデータファイルをリンクする情報に結びつけて情報を整理します。現在あるネットワーク上のほとんどのウェブページはHTMLで作成されています。

CSS

CSSはXMLやHTMLで表現した色・レイアウト・フォントなどの要素を指示する仕様の1つです。