Javascriptの関数について

Javascript初心者です。
画像をスライドさせる関数を作りは同じで、
変数と関数の名前を違うものに変えて実行してみると、
動作しましたが、もっと簡潔に使い回せるような関数の書き方や、
使用方法はないのでしょうか？
アドバイスよろしくお願いします。

Javascript
1
2
3//スライドA
4var pics_src1 = new Array("img1.jpg","img2.jpg");
5var num1 = -1;
6
7slideshow_timer1();
8
9function slideshow_timer1(){
10    if (num1 == 1){
11        num1 = 0;
12    }
13    else {
14        num1 ++;
15    }
16    document.getElementById("A").src=pics_src1[num1];
17    setTimeout("slideshow_timer1()",3000);
18}
19
20
21
22//スライドB
23var pics_src2 = new Array("img3.jpg","img4.jpg");
24var num2 = -1;
25
26slideshow_timer2();
27
28function slideshow_timer2(){
29    if (num2 == 1){
30        num2 = 0;
31    }
32    else {
33        num2 ++;
34    }
35    document.getElementById("B").src=pics_src2[num2];
36    setTimeout("slideshow_timer2()",3000);
37}
38
39
40
41//スライドC
42var pics_src3 = new Array("img5.jpg","img6.jpg");
43var num3 = -1;
44
45slideshow_timer3();
46
47function slideshow_timer3(){
48    if (num3 == 1){
49        num3 = 0;
50    }
51    else {
52        num3 ++;
53    }
54    document.getElementById("C").src=pics_src3[num];
55    setTimeout("slideshow_timer3()",3000);
56}
57

H40831

2020/02/03 15:52

コードブロック機能を正しく使って、きれいな質問文に修正してください

mi...

2020/02/03 16:08

せっかく読んで頂いたのに大変失礼致しました。早速編集させて頂きましたので、よろしくお願い致します。

miyabi_pudding

2020/02/03 16:11

デベロッパーツールでコンソールを見た時、何かエラー起きてませんか？あと、一つ目と二つ目のコードは、同じファイル内に書いているんでしょうか？？

mi...

2020/02/03 23:10

コンソールには、 [Failed to load resource: the server responded with a status of 404 (Not Found)] と出ています。同じファイル内に書いています。外部ファイルで管理する方がいいと書いていましたのでそのようにしています。ただ、CSSファイルのような感じで管理する方法でいいのでしょうか？本当に何も知らずで申し訳ないです。

m.ts10806

2020/02/04 00:45

>使い方の勉強不足で配慮にかけてしまい申し訳ございません。コードを書き直しましたのでよろしくお願いいたします。いきなりなんの話でしょうか。初めて読んだ私にはさっぱり。要件として必要な文章ですか？

mi...

2020/02/04 00:55

修正しましたのでよろしくお願いします????

miyabi_pudding

2020/02/04 01:36 編集

質問編集前と後で、要件がまるで変わってます。私の修正依頼コメントは、同じ関数名を同じファイルに定義してたら、そらエラー起きるでしょ、って意味で質問してました。（つまり、質問内容変わったおかげで、エラーの内容も変わってしまってるってこと）質問修正はかまいませんが、そういう要件がまるで変わる修正は、回答が無駄打ちになるような事態を招いたりのトラブルの原因になるのでやめましょう。（要件が変わる時は、いったんその質問は解決済みにして、別質問を立てるべき）では、修正後の要件ってことでよろしいですか？

mi...

2020/02/04 04:16

大変ご迷惑をおかけいたしました。最初から言いたかったことは今現在ご質問させていただいていることです。伝え方が拙く本当に申し訳ございません。以後このようなことが内容に気をつけます。

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

回答3件

ベストアンサー

最終の質問状態に対して回答します。

こうした方がいいんじゃないか、って部分はソースコードのコメントにて記載しています。
コツは、同じような処理は、変数や引数をうまく使って、一回しか書かない、です。

また、修正依頼コメントで記載ありましたエラーについては、画像が見つかりません、といったエラーとお見受けしますので、
画像に対するパスなど、ご確認ください。

html
1<!DOCTYPE html>
2<html lang="ja">
3<head>
4<meta charset="UTF-8">
5</head>
6<body>
7
8<img id="A">
9<img id="B">
10<img id="C">
11
12<script>
13// スライドのデータ定義
14// スライドデータ郡を処理しやすくするために配列に格納
15const pic_srcs = [
16  {
17    target: 'A',
18    srcs: ['img1.jpg', 'img2.jpg'],
19    count: 0
20  },
21  pics_src2 = {
22    target: 'B',
23    srcs: ['img3.jpg', 'img4.jpg'],
24    // わざわざ-1スタートにする必要がないので、0に変更（意図があるならコメントください）
25    count: 0
26  },
27  pics_src3 = {
28    target: 'C',
29    srcs: ['img5.jpg', 'img6.jpg'],
30    // わざわざ-1スタートにする必要がないので、0に変更（意図があるならコメントください）
31    count: 0
32  }
33];
34
35// インターバルで実行する関数を定義（同じ処理しかしていないので、一個しか定義しない）
36function slideshow_func() {
37  // スライドショーの配列データを走査
38  pic_srcs.forEach(v => {
39    // カウントが、各srcsの数まで達していたら、0に戻し、それ以外なら、+1した数値を代入する（これにより、画像の数が増えても対応できる）
40    // わかりずらかったら、ifブロック形式にします
41    v.count = v.count + 1 === v.srcs.length ? 0 : v.count + 1;
42    /* // 下記がifブロックバージョンです。
43    if (v.count + 1 === v.srcs.length) {
44      v.count = 0;
45    } else {
46      v.count++;
47    }
48    */
49    document.getElementById(v.target).src = v.srcs[v.count];
50  });
51}
52// 今回の場合は、setTimeoutを再起的に実行するより、一発目は実行した上で、setIntervalで繰り返し実行したほうがよさげ
53// 一発目実行
54slideshow_func();
55// あとは繰り返し
56setInterval(slideshow_func, 3000);
57
58</script>
59</body>
60</html>

投稿2020/02/04 02:09

編集2020/02/04 08:57

miyabi_pudding

総合スコア9528

mi...

2020/02/04 04:34

至らない点が多くございましたが、最後まで大変ご丁寧に教えてくださり本当にありがとうございます。自分でもわかるような表現なのでより理解を深めていけるように早速しっかりと理解したいと思います。本当にありがとうございました。

mi...

2020/02/04 04:59

大変申し訳ないのですが、この部分をifブロック形式にしていただくと、どのようになるのか教えていただいてもよろしいでしょうか？ function slideshow_func() { // スライドショーの配列データを走査 pic_srcs.forEach(v => { // カウントが、各srcsの数まで達していたら、0に戻し、それ以外なら、+1した数値を代入する（これにより、画像の数が増えても対応できる） // わかりずらかったら、ifブロック形式にします v.count = v.count + 1 === v.srcs.length ? 0 : v.count + 1; document.getElementById(v.target).src = v.srcs[v.count]; }); }

miyabi_pudding

2020/02/04 08:58

回答のコードに、複数行コメントの中にて記載しました。

mi...

2020/02/04 09:49

凄くわかり易く書いていただき本当にありがとうございます！！質問の件以外にも色々とご指導頂き本当にありがとうございます！！是非また疑問がありましたらよろしくお願いします????

mi...

2020/02/05 06:19

昨日頂いたベストアンサーを自分なりに解読してみようと色々調べてみたのですが、「v」がどういう役割なのかが全くわからないのです。もしよろしければ教えていただけませんでしょうか？

miyabi_pudding

2020/02/05 06:44

javascript forEach では調べてみましたか？これに関しては、"配列"というデータ形式を先に理解しないと、わからないと思うので、上記のワードで調べてみましょう。（forEachという関数は、配列の処理を行う関数なので、上記ワードで同時に学べると思います。）

miyabi_pudding

2020/02/05 06:45

そして、forEachに関しては、本件から外れてしまうので、ご自分で調べて、どうしてもわからなければ、別質問立てましょう。

mi...

2020/02/21 04:10

ありがとうございます。早速for Each分調べてみました！本当に色々ご丁寧にありがとうございます。

miyabi_pudding

2020/02/21 04:28

forEachです。 forとEachの間にスペースを入れると全く意味が変わってきてしまいます。（単語ではなく、あくまでメソッド名なので）細かいですが気を付けましょう。（プログラミングの世界ではその小さいミスが命取りになる）

mi...

2020/02/21 04:42

ご指摘ありがとうございます。細かい部分にも気をつけれるようにならないとダメですね！いつも本当にご丁寧にご指導くださりありがとうございます。

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

2020/02/04 09:54の編集版に向けて回答します。
使い回しをしたいということですが、将来的に画像が2つだけという保証もないかと思いますので多少手厚く書いています。その分コード量は多くなっています。

js
1/**
2 * 画像を一定時間ごとに切り替えて表示する
3 */
4class SlideShow {
5  /**
6   * @param {array}  images      <img src=""> に指定できる画像への path の配列
7   * @param {number} interval    切り替えの間隔 (ミリ秒)
8   * @param {string} targetQuery 切り替え表示させる <img> を指定するセレクタ
9   */
10  constructor(images, interval, targetQuery) {
11    if(!images || images.length === 0) {
12      throw new Error(`Invalid images`);
13    }
14    this.targetDOM = document.querySelector(targetQuery);
15    if(!this.targetDOM) {
16      throw new Error(`DOM ${targetQuery} not exists`);
17    }
18    this.targetDOM.src = images[0];
19
20    this.images = images;
21    this.currentIndex = 0;
22
23    this.interval = interval;
24    this.intervalId = null;
25  }
26
27  /**
28   * 切り替え表示開始
29   * 連続でコールした場合、既存の interval を中断してから実行する
30   */
31  start() {
32    if(this.intervalId !== null) this.end();
33    this.intervalId = window.setInterval(() => {
34      // this.images の中で、次に表示する画像の位置を示す index
35      // this.images.length と等しくなったら、 0 に戻す
36      let index = this.currentIndex + 1;
37      if(index >= this.images.length) index = 0;
38
39      this.targetDOM.src = this.images[index];
40      this.currentIndex = index;
41    }, this.interval);
42  }
43
44  /// 切り替え表示中断
45  stop() {
46    window.clearInterval(this.intervalId);
47    this.intervalId = null;
48  }
49}
50
51const slideShowForA = new SlideShow(['img1.jpg', 'img2.jpg'], 3000, '#A');
52const slideShowForB = new SlideShow(['img3.jpg', 'img4.jpg'], 3000, '#B');
53const slideShowForC = new SlideShow(['img5.jpg', 'img6.jpg'], 3000, '#C');
54
55slideShowForA.start();
56slideShowForB.start();
57slideShowForC.start();

投稿2020/02/04 01:55

編集2020/02/04 02:17

thyda.eiqau

総合スコア2982

thyda.eiqau

2020/02/04 02:17 編集

stopの中でintervalIdをnullにし忘れたな　→　修正済み

mi...

2020/02/04 04:31

大変ご丁寧にありがとうございます。みたことのない表現の仕方が多々あり、まだまだ自分の勉強の足りなさを痛感しております。一つずつ咀嚼しながら勉強させていただきます。本当にありがとうございました。

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

ソースを動かしてみたわけではないのですが、
これは明らかに間違い。
setTimeout("slideshow_timer()",3000);

setTimeoutの第1引数は、文字列ではなく関数リテラルを渡します。
setTimeout(slideshow_timer, 3000);

投稿2020/02/03 16:16

technocore

総合スコア7200

raccy

2020/02/03 22:10

https://developer.mozilla.org/ja/docs/Web/API/WindowTimers/setTimeout evalと同等の動きになるので推奨されていない書き方ですが、JavaScriptのコードである文字列を渡すことはできます。文字列をevalに渡すような動作になり、この部分で問題は起きません。

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

あなたの回答