Go to Qiita Advent Calendar 2024 Top
LoginSignup
limemint
@limemint (斉藤 貴博)

Are you sure you want to delete the question?

Leaving a resolved question undeleted may help others!

同じような処理のまとめ方について

Q&A

Closed

JavaScript

解決したいこと

javascriptで同じような処理をしていくコードを書いているのですが、中身が少しだけ違います。
こういう場合、同じ処理はまとめられそうなのですが、どのようにまとめたらよいのかわかりません。

以下のコードは、数字の1を押したら1がhtmlに表示され、2を押したら同様に2が反映されるものなのですが、
コードの4行目の最後の方にある「1」と「2」が違うだけで他は同じものになっています。
ここだけの違いしかないコードをどのようにひとまとめにしたらいいのでしょうか？

ぜひアドバイスいただけたらと思います。
よろしくお願いいたします。

該当するソースコード

// 1を押した時
function btnOneClick() {
    target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':','');
    const newDisplay = base.substr(1,3)+ '1';
    const a = newDisplay.slice(0,2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  };
// 2を押した時
  function btnTwoClick() {
    target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':','');
    const newDisplay = base.substr(1,3)+ '2';
    const a = newDisplay.slice(0,2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  };
  


### 自分で試したこと
ここに問題・エラーに対して試したことを記載してください。
0

7Answer

Verclene
@Verclene

普通にこれではダメです？

function btnClick(str) {
    target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':','');
    const newDisplay = base.substr(1,3)+ str; // 型チェックとかいるなら自前で
    const a = newDisplay.slice(0,2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  };
2Like

Comments

  1. @limemint

    Questioner

    ご回答いただき感謝します。　
    変わったのは1行目と4行目の後ろですよね？
    ご助言に従い、やってみたところ、私のやり方がいけなかったのだと思いますが、動きませんでした。何がいけなかったのでしょうか？

  2. @Verclene

    変えた内容としては自分の欲しい出力にしたいところだけ引数strにしてるだけです.
    btnClickを呼ぶ側で引数を渡してあげれば良いです.

    targetをグローバルで使ってたりして変なことになってる場合はconstでもつけてやってください.

    上手に処理をまとめるには共通化はもちろんですが,この値Aを受け取って値Bを返す,という目的がはっきりしていることが重要です.
Verclene
@Verclene

自分もサッと書いた手前申し訳ないのですが,現状の回答のつき方だと初学者の方は混乱すると思いますのでもうちょっと書きます.

背景

以前の質問において以下のようなフォームがあり,

<div id="timedisplay">00:00</div>
<form>
    <input type="button" id="number1" value="1" onclick="btnOneClick();">
    <input type="button" id="number2" value="2" onclick="btnTwoClick();">
    <input type="button" id="number3" value="3" onclick="btnThreeClick();"><br>
    <input type="button" id="number4" value="4" onclick="btnFourClick();">
    <input type="button" id="number5" value="5" onclick="btnFiveClick();">
    <input type="button" id="number6" value="6" onclick="btnSixClick();"><br>
    <input type="button" id="number7" value="7" onclick="btnSevenClick();">
    <input type="button" id="number8" value="8" onclick="btnEightClick();">
    <input type="button" id="number9" value="9" onclick="btnNineClick();"><br>
    <input type="button" id="number0" value="0" onclick="btnZeroClick();"><br>
</form>

そのうえで質問文のような無限にある関数をどうすべきかという内容です.

    function btnOneClick() {
        target = document.getElementById('timedisplay');
        const base = target.innerText.replace(':', '');
        const newDisplay = base.substr(1, 3) + '1';
        const a = newDisplay.slice(0, 2);
        const b = ':';
        const c = newDisplay.slice(2);
        let nnDisplay = a + b + c;
        console.log(nnDisplay);
        document.getElementById('timedisplay').textContent = nnDisplay;
        target.innerText = nnDisplay;
    };

共通化方法

引数をとる関数を作る

処理のメインを何回も書くのは面倒なので関数化します.
私が最初に書いたやつです.
引数strを取って#timedisplayに追記します.
処理内容はほとんどオリジナルから変えていないので省略します.

function btnClick(str) {
    const target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':','');
    const newDisplay = base.substr(1,3)+ str; // 型チェックとかいるなら自前で
    const a = newDisplay.slice(0,2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  };

そのうえでonclick属性側からはこんな感じで引数を渡してやります.

<form>
    <input type="button" id="number1" value="1" onclick="btnClick(1);">
    <input type="button" id="number2" value="2" onclick="btnClick(2);">
    <input type="button" id="number3" value="3" onclick="btnClick(3);"><br>
    <input type="button" id="number4" value="4" onclick="btnClick(4);">
    <input type="button" id="number5" value="5" onclick="btnClick(5);">
    <input type="button" id="number6" value="6" onclick="btnClick(6);"><br>
    <input type="button" id="number7" value="7" onclick="btnClick(7);">
    <input type="button" id="number8" value="8" onclick="btnClick(8);">
    <input type="button" id="number9" value="9" onclick="btnClick(9);"><br>
    <input type="button" id="number0" value="0" onclick="btnClick(0);"><br>
</form>

なお引数がなんなのかという内容は,わざわざここに書くよりその辺の解説を見たほうが良いです.

イベントハンドラでonclickすら省略

こうなると毎回onclickを書くのも面倒なので,イベントハンドラをまとめて登録してやります.
@think49 さんが書いてるやつです.

    document.querySelectorAll('input[type=button]').forEach(button=> {
        button.addEventListener('click', event => btnClick(event.target.value));
    });

やっていることとしては

  1. document.querySelectorAll('input[type=button]')でbuttonを全部取り出す(input[type=button]は他のボタンも捕捉してしまいますので,適宜調整してください)
  2. .forEach(button => { ... }で,取り出したbuttonのすべてに適用する処理を書く
  3. buttonをclickしたときのイベントハンドラevent => btnClick(event.target.value)をaddEvenetListener登録する

ここではざっくりとした解説にとどめますが,イベントハンドラはeventという引数を受け取るお約束があり,event.targetにクリックした要素の情報が入っています.
そのうえでevent.target.valueでbuttonのvalue属性値を読みだしてbtnClickを呼びだすという流れです.

querySelectorAllは下手なセレクタだと余計なbuttonも捕捉してしまったり,addEventListenerのthisバインドなんかもややこしい仕様があったりするので,慣れてから使うと良いでしょう.

@tonberry1050
このくらいならここで引数指定してもいいですが、eventは作法みたいなものです。

onclick属性には実行する文を直接入力できます.eventは勝手に渡されません.

2Like

Comments

  1. @limemint

    Questioner

    さらに詳しい解説ありがとうございます。
    まるでスクール1回分のレッスンのような丁寧な回答で感謝します。
    考え方を順を追って解説してくださったのでよくわかりました。
    早速「イベントハンドラでonclickすら省略」の手前までをコピペで実行してみた（codepen;)のですが、うまく動きませんでした。こんなに詳細に教えてくださったのにうまく実装できない自分が不甲斐ないです。原因を究明したいと思います。

  3. @limemint

    Questioner

    ありがとうございます！！
    ちゃんと動くようになりました。
    もう何から何まで、至れり尽くせりのご対応をしていただきまして、お礼の言葉がもう見つかりません…。いいねボタンを100回くらい押したいくらいです。
    本当にありがとうございました。
HalHarada
@HalHarada

@Verclene　さんに賛成です。

document.getElementById('timedisplay');

function btnClick(str) {

付け加えれば、折角、関数にするなら、外界のオブジェクトを関数内で持ちるのではなく、

btnClick(1,　'timedisplay')

function btnClick(str, id) {
とかするとよいのでは

1Like

Comments

  1. @limemint

    Questioner

    ご回答いただきましてありがとうございます。
    追加の情報までいただき感謝します。
    早速やってみます。
think49
@think49

@limemint さん

こういう場合、同じ処理はまとめられそうなのですが、どのようにまとめたらよいのかわかりません。

単純に対策するなら、「異なる値」を「変数」や「オブジェクトのプロパティ」に格納してください。
addEventListener には第二引数がオブジェクトだった場合、第二引数に「this値」を束縛する機能があります。

<input type="button" value="1" />
<input type="button" value="2" />
<script>
  'use strict';
  function handleClick(event) {
    console.log(this.i);
  }

  document.querySelectorAll('input[type=button]').forEach((button, i) => button.addEventListener('click', {
    i: i + 1,
    handleEvent: handleClick
  }));
</script>

ただし、この場合は handleEvent を使うまでもありません。
前提が「1を押した時」や「2を押した時]ということは、「押された要素」に "1" や "2" という情報があるということです。

<input type="button" value="1" />
<input type="button" value="2" />
<script>
  'use strict';
  function handleClick(event) {
    console.log(event.target.value);
  }

  document.querySelectorAll('input[type=button]').forEach((button, i) => button.addEventListener('click', handleClick, false));
</script>
1Like

Comments

  1. @limemint

    Questioner

    ご回答いただき感謝します。
    私としては「ここからここまでを一つの関数にして…」みたいな感じでできるのかなと安易に考えていたのですが、結構難しいのですね。
    いただいた回答をじっくり読み込んで理解できるようになります。
tonberry1050
@tonberry1050

携帯で見ていて間違ってたら悪いのですが、違うのはconst newDisplay = base.substr(1,3)+ '1';の部分だけでしょうか。でしたらこうです。

// こっちの2つが呼び出しに使う方
const onClickOneBtn = () => {
  return updateDisplay('1');
};
const onClickTwoBtn = () => {
  return updateDisplay('2');
};

const updateDisplay = (num) => {
    target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':','');
    const newDisplay = base.substr(1,3)+ num;//違う部分だけifにしたり引数を使う
    const a = newDisplay.slice(0,2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  };

多分ボタンのクリックイベントに使うのかと思いますが、クリックイベントと実際に処理を行う関数は分けることが多いです。

1Like

Comments

  1. @tonberry1050

    以下も同じ意味です、このくらいならここで引数指定してもいいですが、eventは作法みたいなものです。
    ```html
    <div onclick="onClickOneBtn(event)"></div>
    ```

    多分求めてたのはこんくらいシンプルな話なのかなと、、、

  2. @limemint

    Questioner

    ご回答ありがとうございます。
    シンプルで見やすいです。
    試しにコピペでcodepen;使ってやってみたのですが、動かなかったのは私の他のコードが何かを阻害しているのでしょうか？
    いろいろ試してみようと思っているのですが、取り急ぎお礼をお伝えいたします。
think49
@think49

解決しているようですが、補足しておきます。
@Verclene さんの下記コメントですが、

onclick属性には実行する文を直接入力できます.eventは勝手に渡されません.

onclick属性で書かれたコードのスコープ上に event はあります。

<script>
  'use strict';
  function btnClick(event) {
    const target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':', '');
    const newDisplay = base.substr(1, 3) + event.target.value;
    const a = newDisplay.slice(0, 2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  }
</script>
<div id="timedisplay">00:00</div>
<form>
  <input type="button" id="number1" value="1" onclick="btnClick(event);">
  <input type="button" id="number2" value="2" onclick="btnClick(event);">
  <input type="button" id="number3" value="3" onclick="btnClick(event);"><br>
  <input type="button" id="number4" value="4" onclick="btnClick(event);">
  <input type="button" id="number5" value="5" onclick="btnClick(event);">
  <input type="button" id="number6" value="6" onclick="btnClick(event);"><br>
  <input type="button" id="number7" value="7" onclick="btnClick(event);">
  <input type="button" id="number8" value="8" onclick="btnClick(event);">
  <input type="button" id="number9" value="9" onclick="btnClick(event);"><br>
  <input type="button" id="number0" value="0" onclick="btnClick(event);"><br>
</form>

ここで定義された関数 btnClick は addEventListener の第二引数に指定可能な関数なので、addEventListener に置き換え可能、というのが私の回答の趣旨になります。

<div id="timedisplay">00:00</div>
<form id="sample">
  <input type="button" id="number1" value="1">
  <input type="button" id="number2" value="2">
  <input type="button" id="number3" value="3"><br>
  <input type="button" id="number4" value="4">
  <input type="button" id="number5" value="5">
  <input type="button" id="number6" value="6"><br>
  <input type="button" id="number7" value="7">
  <input type="button" id="number8" value="8">
  <input type="button" id="number9" value="9"><br>
  <input type="button" id="number0" value="0"><br>
</form>
<script>
  'use strict';
  {
    const btnClick = event => {
      const target = document.getElementById('timedisplay');
      const base = target.innerText.replace(':', '');
      const newDisplay = base.substr(1, 3) + event.target.value;
      const a = newDisplay.slice(0, 2);
      const b = ':';
      const c = newDisplay.slice(2);
      let nnDisplay = a + b + c;
      console.log(nnDisplay);
      document.getElementById('timedisplay').textContent = nnDisplay;
      target.innerText = nnDisplay;
    };

    document.querySelectorAll('#sample>input[type=button]').forEach(input => input.addEventListener('click', btnClick, false));
  }
</script>

更に発展させると、イベントバブリングの仕組みから親要素でイベント発火を検知できるので、form要素ノードに仕掛ける事で addEventListener が一回の実行で済みます。

<div id="timedisplay">00:00</div>
<form id="sample">
  <input type="button" id="number1" value="1">
  <input type="button" id="number2" value="2">
  <input type="button" id="number3" value="3"><br>
  <input type="button" id="number4" value="4">
  <input type="button" id="number5" value="5">
  <input type="button" id="number6" value="6"><br>
  <input type="button" id="number7" value="7">
  <input type="button" id="number8" value="8">
  <input type="button" id="number9" value="9"><br>
  <input type="button" id="number0" value="0"><br>
</form>
<script>
  'use strict';
  document.getElementById('sample').addEventListener('click', event => {
    const input = event.target;

    if (input.tagName !== 'INPUT') return;

    const target = document.getElementById('timedisplay');
    const base = target.innerText.replace(':', '');
    const newDisplay = base.substr(1, 3) + input.value;
    const a = newDisplay.slice(0, 2);
    const b = ':';
    const c = newDisplay.slice(2);
    let nnDisplay = a + b + c;
    console.log(nnDisplay);
    document.getElementById('timedisplay').textContent = nnDisplay;
    target.innerText = nnDisplay;
  }, false);
</script>

更にコードをスリムに。

<div id="timedisplay">00:00</div>
<form id="sample">
  <input type="button" id="number1" value="1">
  <input type="button" id="number2" value="2">
  <input type="button" id="number3" value="3"><br>
  <input type="button" id="number4" value="4">
  <input type="button" id="number5" value="5">
  <input type="button" id="number6" value="6"><br>
  <input type="button" id="number7" value="7">
  <input type="button" id="number8" value="8">
  <input type="button" id="number9" value="9"><br>
  <input type="button" id="number0" value="0"><br>
</form>
<script>
  'use strict';
    document.getElementById('sample').addEventListener('click', event => {
      const input = event.target;

      if (input.tagName !== 'INPUT') return;

      const time = document.getElementById('timedisplay');
      const string = time.textContent;
      time.textContent = string[1] + string[3] + ':' + string[4] + input.value;
    }, false);
</script>

元コードの textContent と innerText に同じ文字列を各々代入しているコードが謎です…。

1Like

Comments

  1. @limemint

    Questioner

    詳細な追加解説をありがとうございます！
    順を追って解説してくださったので理解が深まりました。
    もしこちらのコメント見ていただけたら、<script>の後にある 'use strict';　について教えていただけないでしょうか？
    これはどういう意味を持ったコードになるのでしょうか？
think49
@think49

@limemint さん

もしこちらのコメント見ていただけたら、<script> の後にある 'use strict';　について教えていただけないでしょうか？

"use strict" でGoogle検索してみてください。

Strict Modeはコーディングの落とし穴になる機能を封印します。
JavaScriptは後方互換性を担保する必要がある為、従来のモード(非Strict Mode、Sloppy Mode)でも書けるようになっています。

後になって過去質問のコードを見ましたが、textContent と innerText で同じ文字列を代入している処理が無駄だったり、addEventLitener でDOMContentLoadedイベントを監視しながら、clickイベントはonclick属性で監視したり、とちぐはぐな部分がいくつか見られます。

頂いた回答のコードを取り込んでいくと、別々のコードを組み合わせるのでちぐはぐになりがちですが、学習目的なのでしたら、一つ一つの関数/コードの意味を調べて理解しておくことが重要だと思います。

1Like

Comments

  1. @limemint

    Questioner

    ご回答ありがとうございます。
    自分で検索をするという基本的なことを失念しておりました。大変失礼しました。
    ちぐはぐな部分のご指摘もありがとうございます。そういうところまで指摘して頂ける機会があまりないので、指摘されるまで気が付きませんでした。javascriptをはじめ、htmlもまだわかっていない部分も多く、どしどし指摘してほしいと思っています。しかし、なんとなく動いたからいっかー、と安心しておりました。ご指摘心から感謝いたします。いただいたご指摘をもとにして再度見直しをしたいと思います。

Your answer might help someone💌

LoginSign Up