配列がfor文で回らない、Promiseが機能しないときがある

Question

前提・実現したいこと

現在フロント側にjavascriptを用いて、SNSアプリのようなものを作っています。

ユーザーは他のユーザーの投稿をお気に入り登録でき、お気に入り一覧画面で閲覧できるようにしたいと考えています。

DBには投稿情報（写真管理サイトで管理する写真データのURL含む、一覧表示時に写真管理サイトの該当URLを参照しています）がすべて入ったAコレクション、お気に入り登録した作品についてユーザーIDと投稿ID（Aコレクションで付与される）が入ったBコレクションがあります。

お気に入り一覧画面では

1. function Favorites()
   Bコレクションから投稿IDを取得、favorite_data_id配列に挿入する。
2. function FavoriteDatas()

Aコレクションから全投稿内容を取得、favorite_data_id配列のいずれかとidが一致した投稿をfavorite_datas配列に挿入する。
3. function addToList(now_datas)
引数now_datas配列でfor文を回し、画面上に表示する。

という順序で最初の画面は表示させたいと考えています。

3.function addToList(now_datas)でfavorite_datas配列をそのまま利用しない理由は、絞り込み検索結果表示の際にもaddToList関数を利用したいためです。

発生している問題

配列がfor文で回らないのとPromiseが機能しないときがあるのは別問題だとは思ったのですが、どのような時にfor文で回らないのか、Promiseが機能しないのか判別できなかったので、一度に質問しています。

1. 配列がfor文で回らない

必ず起こります。
3.function addToList(now_datas)で引数now_datas配列が回りません。
Promiseが機能しているときにはfor文の直前でnow_datas配列を取得すると、now_datas配列にデータは入っているのですが、インデックス番号を指定してもundefinedになり、for文は回りません。
2. Promiseが機能しない
時々起こります。
今回初めてPromiseを使用しており、まだPromiseが確実に機能する状態のコードを書けたことないのですが、そもそも3.function addToList(now_datas)到達時にfavorite_datas配列が空の状態です。
2.function FavoriteDatas()到達時にfavorite_data_id配列はセットされています。

どちらかだけでも良いので、教えていただけないでしょうか。

該当のソースコード

      let favorite_data_id = []; // お気に入り登録してあるworkid
      let favorite_numbers = 0; // お気に入り登録してある数
      let favorite_datas = [];  // お気に入り登録してある全作品
      window.onload = allDatas(); // 画面ロード時に実行
      
      function Favorites() {
        const url = '/Favorites'; // 通信先
        const req = new XMLHttpRequest(); // 通信用オブジェクト
        
        req.onreadystatechange = function() {
          if(req.readyState == 4 && req.status == 200) {
            const datas = JSON.parse(req.response);
            for(let i in datas) {
              const data = datas[i];              
              favorite_data_id[favorite_numbers] = data.workid;
              favorite_numbers++;
            }
          }
        }
        req.open('GET', url, true);
        req.send();
      }
      
      // 全データの取得
      function FavoriteDatas() {
        console.log(favorite_data_id);
        const url = '/AllDatas'; // 通信先
        const req = new XMLHttpRequest(); // 通信用オブジェクト
        
        req.onreadystatechange = function() {
          if(req.readyState == 4 && req.status == 200) {
            const datas = JSON.parse(req.response);
            all_datas = datas;
            for(let i in all_datas){
              for(let j in favorite_data_id){
                if(all_datas[i]._id == favorite_data_id[j]){
                  favorite_datas[j] = all_datas[i];
                }
              }
            }           
          }
        }
        req.open('GET', url, true);
        req.send();
      }
      
      // 表示
      function addToList(now_datas) {
        for(let i in now_datas){
          // 表示
        }
      }
      
      function allDatas(){
        new Promise((resolve) => {
          Favorites();
          resolve();
        }).then(() => {
          FavoriteDatas();
        }).then(() => {
          addToList(favorite_datas);
        });
      } 

Comments

[itagagaki]

デバッグをしたことはないのですか？

[Lhankor_Mhy]

とりあえず、返り値のない関数が散見されるのですが、その辺は大丈夫なんですか？

[cerasusflorere]

itagagaki様
デバッグというのはconsole.log()で何か書いて、開発ツールで見ることでしょうか？
それであればしたことはあります。

Lhankor_Mhy様
setTimeoutで処理を遅らせていた時には表示ができていたのですが（Promiseに変更しようといじった結果表示できなくなりました。別のページでは変わらず返り値を設定しなくともsetTimoutで表示できてはいます）、関数には必ず返り値が必要なのでしょうか？

[think49]

直接的な問題ではありませんが、関数名が名詞なので名前から機能が読み取れないのも気になりました。
関数名は動詞にすると良いです。

[Lhankor_Mhy]

> 関数には必ず返り値が必要なのでしょうか？

いえ、返り値を返さないなら undefined が返るだけです。それを想定しているのであれば、問題ないですが、たとえば FavoriteDatas() が undefined を返すのは想定通りなのかな、ということが気になっているという話です。

[cerasusflorere]

think49様
関数名は動詞にするのがマナーとは知りませんでした。教えていただきありがとうございます。

[think49]

@cerasusflorere さん
マナーというより「名前と内容が一致していない」という意味です。

「お気に入りのデータ(FavoriteDatas)」という名前の変数があったとして、格納されている内容を console.log(お気に入りのデータ) で確認したところ、データではなく、関数でした。
「お気に入りのデータ」になにがしかの処理をする関数と思われますが、「お気に入りのデータ」の名前から処理内容が読み取れません。

「お気に入りのデータを取ってきてくれ(fetchFavoriteData)」なら名前から処理内容を想像できます。
("Data" は複数形の為、"s" は不要です)

[cerasusflorere]

think49様
ありがとうございます。今後は動詞で書こうと思います。

Answer 1

ku__ra__geさんのご回答に追加して、thenメソッドのハンドラ関数が値を返していないことを指摘しておきます。

何も返さなかった場合、 then によって返される Promise は undefined の値で解決します。

Promise.prototype.then() - JavaScript | MDN

これはつまり、.then(() => {FavoriteDatas();})は、FavoriteDatasの処理終了を待たずに即座に解決されるということです。

Comments

[cerasusflorere]

回答してくださいありがとうございます。

FavoriteDatas関数に返り値を設定すれば、処理終了を待ってもらえるのでしょうか？
それともなんらかの数値をthenメソッド内でreturnするのでしょうか？

最初のチェーン以降の処理の待ち方は調べたのですが、分からず、特に何もしなくとも大丈夫なものなのかと思っておりました。
教えていただけると幸いです。

[Lhankor_Mhy]

https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Promise/then#return_value

こちらに書いてあるとおり、pending の Promise を返せば、解決まで待つようになります。

ですので、FavoriteDatas() の処理終了時に解決するような Promise を渡せばいいです。
まずは、Promise を返す XMLHttpRequest() のラッパー的な関数を書いてしまった方が楽なような気がしますね。
「Promise XMLHttpRequest」でググれば、参考になるコードが見つかるのではないかと思います。

---

IE11 に対応してないですが、fetch() は Promise を返すので、このようなコードを書く時にかなり楽ができます。
https://developer.mozilla.org/ja/docs/Web/API/fetch

[cerasusflorere]

ありがとうございます。
しっかり勉強したいと思います。

Answer 2

Promiseが機能しない
時々起こります。
今回初めてPromiseを使用しており、まだPromiseが確実に機能する状態のコードを書けたことないのですが、
そもそも3.function addToList(now_datas)到達時にfavorite_datas配列が空の状態です。
2.function FavoriteDatas()到達時にfavorite_data_id配列はセットされています。

Promiseの使い方が間違っているからです。
単純化した例を以下に示します。

// 質問者さんが書いているコード
// これを実行すると、<<B>> → <<A>> の順に処理が実行されます。
// つまりXMLHttpRequestの通信結果を受け取る前に、then(()=>{……の処理が実行されてしまうということです。
new Promise((resolve) => {

  const req = new XMLHttpRequest(); // 通信用オブジェクト
  req.onreadystatechange = function() {
    if(req.readyState == 4 && req.status == 200) {
      console.log("<<A>>");
    }
  }
  req.open('GET', url, true);
  req.send();

  resolve();

}).then(()=>{
  console.log("<<B>>");
});

// 質問者さんが求めているであろう動作を行うコード
// これを実行すると、<<A>> → <<B>> の順に処理が実行されます。
new Promise((resolve) => {

  const req = new XMLHttpRequest(); // 通信用オブジェクト
  req.onreadystatechange = function() {
    if(req.readyState == 4 && req.status == 200) {
      console.log("<<A>>");
      resolve();
    }
  }
  req.open('GET', url, true);
  req.send();

}).then(()=>{
  console.log("<<B>>");
});

Promiseの概念を理解できてないようなので、Promiseを学習してください。
Promise - JavaScript | MDN

あとjavascriptで配列の反復処理はfor～ofを使ってください。
for～inは配列の反復処理を正しく行えるとは限りません。

配列の繰り返しと for...in

注: for...in はインデックスの順序が重要となる 配列 の繰り返しには使うべきではありません。

Comments

[cerasusflorere]

回答してくださり、ありがとうございます。

Promiseは機能するようになりました。
関数実行終了まで待ってくれるわけではないのですね。勉強します。

for...ofで実装してみたのですが、valueでないためか、
for(let data of now_datas){
console.log(data);
}
としてもdataがundefinedになってしまいます。
forEach文にしてもPromiseからの実行では回りません。
質問では書かなかったのですが、投稿にteratailでいうタグのようなものをつけており、タグ検索を行うのですが、この時は回ります。

質問の内容が変わってしまうので、別質問に分けようと思います。
ありがとうございました。