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

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

ただいまの
回答率

88.03%

javascriptの簡単なコードなのですが、非効率になってしまうのを効率的にしたいです。

解決済

回答 3

投稿

  • 評価
  • クリップ 0
  • VIEW 461

score 33

現在練習用のサイトを作成しておりまして、javascriptを使っているのですが非効率的で困っています。もっと効率良くかけるのではないかと考えているのですが、ちゃんと動くコードにするだけでとても時間がかかってしまいました。

実現したい事はラジオボタンでAタイプを選択した状態の場合、セレクトボックスの結果を<p id="a_type">に出力してラジオボタンでBタイプを選択した場合セレクトボックスの結果を<p id="b_type">に出力するとゆうことです。

<!--ここでTYPEを選択します-->
<form name="form1" action="">
  <input type="radio" id="radio1" name="page" required checked onchange="selected_page();">A-TYPE
  <input type="radio" id="radio2" name="page" onchange="selected_page();">B-TYPE
</form>

<!--上記のTYPEに表示したい項目を選択します-->
<form name="form2" action="">
  <select id="Select1" onchange="selected_page();">
    <option>0ページ</option>
    <option>1ページ</option>
    <option>2ページ</option>
    <option>3ページ</option>
    <option>4ページ</option>
  </select>
</form>

<!--ここに結果を表示させます-->
<p id="a_type">0ページ</p>
<p id="b_type">0ページ</p>
#b_type {
  display: none;
}
function selected_page() {
    let check1 = document.getElementById("radio1").checked;
    let check2 = document.getElementById("radio2").checked;

    if (check1 == true) {
        let sel = "a_type";
        let target = document.getElementById(sel);
        target.style.display = "block"
        document.getElementById("b_type").style.display = "none"
        let selindex = document.getElementById("Select1").selectedIndex;
        switch (selindex) {
            default:
                target.innerHTML = "0ページ";
                break;
            case 1:
                target.innerHTML = "1ページ";
                break;
            case 2:
                target.innerHTML = "2ページ";
                break;
            case 3:
                target.innerHTML = "3ページ";
                break;
            case 4:
                target.innerHTML = "4ページ";
                break;
        }
    }
    else if (check2 == true) {
        let sel = "b_type";
        let target = document.getElementById(sel);
        target.style.display = "block"
        document.getElementById("a_type").style.display = "none"
        let selindex = document.getElementById("Select1").selectedIndex;
        switch (selindex) {
            default:
                target.innerHTML = "0ページ";
                break;
            case 1:
                target.innerHTML = "1ページ";
                break;
            case 2:
                target.innerHTML = "2ページ";
                break;
            case 3:
                target.innerHTML = "3ページ";
                break;
            case 4:
                target.innerHTML = "4ページ";
                break;
        }
    }
}


javascriptですが、内容が同じswitch文を二回使ってしまっていたり色々と問題がある気がしています..

お詳しい方だったらどのような感じにするのか教えていただけないでしょうか?

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

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

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

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

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

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

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

checkベストアンサー

+2

<select></select>要素から現在の値を得るのに、.selectedIndexを介しswitchする必要はありません。.valueで直接取れます。
そのため元コードの以下の部分は…

        let selindex = document.getElementById("Select1").selectedIndex;
        switch (selindex) {
            default:
                target.innerHTML = "0ページ";
                break;
            case 1:
                target.innerHTML = "1ページ";
                break;
            case 2:
                target.innerHTML = "2ページ";
                break;
            case 3:
                target.innerHTML = "3ページ";
                break;
            case 4:
                target.innerHTML = "4ページ";
                break;
        }


以下のコードに置き換えられます。

    let page = document.getElementById("Select1").value;
    target.innerHTML = page;


次に、チェックされてたラジオボタンを得るために両者のチェック状態を取得しif文で判定してらっしゃいますが、querySelectorを使って以下のように書くと簡単に現在チェックされている要素を取れます。

    let checked = document.querySelector('[name=page]:checked');


ここまででswitchifを消して共通化でき、大幅にコードが短くなると思いますが、好みもあるでしょうがあと二点。
一つは選択されたradioボタンに対応する表示p要素のidを、jsコード中のハードコードから、html側でdata属性(名前はdata-targetにしました)で指定するようにしました。

  <input type="radio" id="radio1" name="page" required checked onchange="selected_page();" data-target="a_type">
A-TYPE
  <input type="radio" id="radio2" name="page" required onchange="selected_page();" data-target="b_type">B-TYPE


JavaScriptからは、以下のように取り出し・使用します。

    let checked = document.querySelector('[name=page]:checked');
    let targetId = checked.dataset.target;
    let target = document.getElementById(targetId);


二点目は、2つの表示用<p></p>要素の表示非表示の切り替えを.style.displayへの値の代入で行ってらっしゃいますが、これをdisplay: none;だけ記述した非表示CSSクラス(名前はhideなど)を用意して、CSSクラスの付け外しで行うようにしました。2要素なので自己満足なのですが、要素が多くなってもjsコードが複雑化しにくいです。
以下のような感じです。

.hide {
  display: none;
}
<p id="a_type">0ページ</p>
<p id="b_type" class="hide">0ページ</p>
    let target = document.getElementById(targetId);
    document.querySelectorAll('#a_type,#b_type').forEach(elm => {
      elm.classList.add('hide');
    });
    target.classList.remove('hide');


考え方としては、(非表示用クラス.hideを付けることで)一旦すべて隠し、ターゲットのみ外す、という2ステップになります。(#a_type,#b_typeと要素を個別に列挙していますが、あとで要素が増えるとjsコードに追加の修正が要るため、本当は共通クラスをつけるなり、divでラップするなりして、一括指定したほうがいいです)

動作確認コードはこちらになります。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

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

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

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/12/22 12:38

    貴重なご意見ありがとうございます!とても参考になりましたm(_ _)m

    思っていたよりも大分短くできて初めてみるコードも沢山あり勉強になりました!
    ありがとうございましたm(_ _)m

    キャンセル

+1

こんにちは

一例として、以下のようにするかなと思います。

まずHTMLのほうで、各ラジオボタンに value 属性を追加して、その値を a_type および b_type にします。

<input type="radio" id="radio1" name="page" value="a_type" required checked onchange="selected_page();">A-TYPE
<input type="radio" id="radio2" name="page" value="b_type" onchange="selected_page();">B-TYPE

その上で、 関数 selected_page() を以下のように修正します。

function selected_page() {  
  const targetId = document.forms[0].page.value;  
  const target = document.getElementById(targetId);
  const selindex = document.getElementById("Select1").selectedIndex;
  target.style.display = "block";
  target.innerHTML = `${selindex}ページ`;

  const otherId = `${targetId === 'a_type' ? 'b' : 'a'}_type`;
  document.getElementById(otherId).style.display = "none";
}

参考になれば幸いです。

追記

shinji709さんのご回答で指摘されているように

const selindex = document.getElementById("Select1").selectedIndex;

target.innerHTML = `${selindex}ページ`;


の部分は、以下の一行で済みます。

target.innerHTML = document.getElementById("Select1").value;


上記のように、代入される右辺がHTMLではない文字列である場合は、 target のtextContent プロパティに代入するように

target.textContent = document.getElementById("Select1").value;


としてもよいです。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

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

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

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

0

同じswitch文を二回使ってしまっていたり、色々と問題がある気がしています

問題に感じているのは、「読み返すと、長くて読みにくい。」では?

同じような処理は、

  1. 関数化できないかを考えます。
  2. 関数の入力、出力 の値を見直します。

例えば、関連する 要素の id 属性を与える方法だと次のようにできそうです。

const formatPage = pagenum => (pagenum<5 ? pagenum : 0) + "ページ";

const getPageName = (showId, hideId, selectedId) => {
  let target = document.getElementById( showId );
  target.style.display = "block"
  document.getElementById( hideId ).style.display = "none";

  let selindex = document.getElementById( selectedId ).selectedIndex;
  target.innerHTML = formatPage(selindex);

}

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

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

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

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

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

  • ただいまの回答率 88.03%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

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