JavascriptのPromiseについて

Question

Javascript（Nodejs）とcheerio-httpcliを使ってスクレイピングの処理を書いているのですが、実行順序がコントロールできずに途中でプログラムが終わってしまいます。

var loopProcess = function(callback) {
    return new Promise(function(resolve, reject) {
        for (i = 0; i < _keyword_array.length; i++) {
	    console.log("Inside Loop")
	    // 指定された検索エンジンによって処理を分ける
            if (_search_engine == "google") {
                goGoogle(_keyword_array[i], _user_agent, _check_keyword)
            }
            else {
                goYahoo(_keyword_array[i], _user_agent, _check_keyword)
            }
        }
    })
}

// 結果を出力する
var finishProcess = function() {
    return new Promise(function(resolve, reject) {
        _result_html += "</table>"
        _result_area.innerHTML += _result_html
        console.log("finish end process")
    })
}

loopProcess().then(finishProcess())

_keyword_arrayには3件のテストデータが入っており、for文は3回回らなければなりません。
しかし、
Inside Loopのログが１回だけ出て、その後は、finish end processと出て終わってしまいます。
何か書き方が悪いことはわかるのですが、どこをどう直せばいいのかがわかりません。

よろしくお願いします。

Answer 1

何か書き方が悪いことはわかるのですが、どこをどう直せばいいのかがわかりません。

コードを見る限りPromiseについて分かっていないで書いているようなので、
まずはPromiseについてまずは学ぶと良いのではないでしょうか。
そして、その後、必要であれば質問を再度されていはいかがですか？

Comments

[kazzzstudio]

まさにおっしゃる通りであることはわかっていたのですが、納期が迫っていたため止むを得ずお聞きしたという次第です。なんとか動くコードが作れたのでよかったです。これから勉強します。

Answer 2

各変数の中身が分からないので、
回答者が最後まで面倒みるのは不可能です。

Promiseの書き方は試行錯誤の後が見れますが、sousukeさんの仰るように
何か簡単なresolveやrejectを発火するコードを書いて動作を確かめてみてください。

さて、それ以外のどう見ても駄目そうな箇所を幾つか突っ込みます。

• var loopProcess = function(callback)このコールバック関数は何？
• goGoogleとgoYahooって非同期な処理じゃないの？
• .then(finishProcess())何も待たずに発火して評価開始してる件

---

var loopProcess = function(callback)このコールバック関数は何？

試行錯誤の後でしょうね。
これは普通の非同期処理を書く時のやり方で、
新しいPromiseを使った非同期処理の時は不要です。

コードを見せる時に削っておきます。

goGoogleとgoYahooって非同期な処理じゃないの？

質問文に「Inside Loopのログが１回だけ出て」という記載があるのでほぼ確定でしょう。
goGoogleがコールバック関数を要求するのか、はたまたPromiseを返すのかでまた分岐しますが、
全て実行完了するのを待ってからresolve(value)にするべきでしょう。

もっと言うなら、お外のスコープで宣言した_result_htmlを書き換える設計は死ぬほどダサいです。
resolve(value)の中身を_result_htmlとして実行するべきでしょう。

更に一手進めてasync / awaitを使ってリファクタリングすればこんな感じになると想定されます。

// loopProcess :: Object -> Promise
const loopProcess = async options => {
  const {search, keywords, user_agent, check_keyword} = options
  const results = []
  for (const keyword of keywords) {
    console.log("Inside Loop")
    // なんでキーワード全て同じ検索エンジンになるの？外からもってきなさい
    // searchはPromiseを返す関数と決め打ちしてawaitで待つ
    const result = await search(keyword, user_agent, check_keyword)
    results.push(result)
  }
  return results
}

// 恐らくgoGoogleとgoYahooもリファクタリングの必要あり
const search = _search_engine == "google" ? goGoogle : goYahoo
const options = {
  search,
  keywords: _keyword_array,
  user_agent: _user_agent,
  check_keyword: _check_keyword
}
loopProcess(options)

---

.then(finishProcess())何も待たずに発火して評価開始してる件

発火させてはいけません。
前のPromiseのresolve(value)の第一引数(value)がthenの第一引数に格納されて発火される作りになっているからです。

このように記述してください。

// Promiseである必要はないので普通の関数にした
finishProcess = (html, area, results) => {
  // TODO: resultsを加工する
  const result_html = toResultHTML(results)
  area.innerHTML += html + result_html + "</table>"
  console.log("finish end process")
}

// options変数は前章を参考
// _result_htmlと_result_areaはfn.prototype.bindを使って束縛する
loopProcess(options)
  .then(finishProcess.bind(null, _result_html, _result_area))

Comments

[kazzzstudio]

わかりやすいご説明ありがとうございます！

Answer 3

resolve()とreject()してないですね…。
promiseは簡単にいうと
・resolveしたときにthenコールバックに入る
・rejectしたときにcatchコールバックに入る
というものですよ。

両方とも呼んでないので意味をなさないコードになっています。
使い方を調べた方がよろしいかと。

Comments

[kazzzstudio]

勉強が足りませんでした。もっとよく調べます。