2017-08-27 6 views
2

リクエストパッケージでイメージを一括ダウンロードする機能を記述しています。 ダウンロード部分は動作しますが、Promise配列にパラメータを渡す際に問題があります。 ループ内にパラメータを渡すプロミス

私はこれを実行する

function downloadImages(data) { 
 
    var promises = []; 
 
    var promise, local, id, url; 
 

 
    for (var i in data) { 
 
    (function(i) { 
 
     local = "public/images/".concat(data[i].id, ".png"); 
 
     url = data[i].img_url 
 
     id = data[i].id 
 

 
     promise = request.get({ 
 
     url: url, 
 
     local: local, 
 
     id: id, 
 
     encoding: 'binary' 
 
     }, function(err, res) { 
 
     if (!err && res.statusCode === 200) { 
 
      fs.writeFile(local, res.body, { 
 
      encoding: 'binary' 
 
      }, (err) => { 
 
      if (!err) { 
 
       doSomething() 
 
      } else { 
 
       console.log("Error Downloading image") 
 
      } 
 
      }) 
 
     } 
 
     }) 
 
     promises.push(promise) 
 
    })(i) 
 
    } 
 
    Promise.all(promises); 
 
}

、最後のエントリへの配列解決のすべてのパラメータ、それをダウンロードしています(data.length)回。

私はいくつかのことを試しましたが、解決策に近づくことはできません。何か基本的なイムが間違っているのか、それとも何か簡単なことがありますかいくつかの助けに非常に感謝しています!

+1

あなたは 'i'で行ったように、変数' local'、 'id'、' url'をループ本体の中で宣言するのはどうですか? – Bergi

+0

'request'は実際に約束を返しますか?あなたは[このような約束でリクエストを包む]必要はありません(https://stackoverflow.com/questions/41412512/node-js-promise-request-return)?特に 'request-promise'と呼ばれる別のパッケージがnpmにあることを知っているからです。 – Andy

+1

'data'は配列です:' for ... in'列挙型を使わないでください! '(https://stackoverflow.com/q/500504/1048572)代わりに 'map'を使うと、あなたの問題を最も簡単に解決できます。 – Bergi

答えて

1

私はこのように単純化することをお勧めしたい:

// make promisified version of fs.writeFile() 
fs.writeFileAsync = function(fname, data, options) { 
    return new Promise((resolve, reject) => { 
     fs.writeFile(fname, data, options, err => { 
      if (err) { 
       reject(err); 
      } else { 
       resolve(); 
      } 
     }); 
    }); 
} 

function downloadImages(data) { 
    let promises = data.map(obj => { 
     let local = "public/images/".concat(obj.id, ".png"); 
     let url = obj.img_url; 
     let id = obj.id; 
     return request.get({url, local, id, encoding: 'binary'}).then(imgData => { 
      return fs.writeFileAsync(local, imgData, {encoding: 'binary'}).then(doSomething).catch(err => { 
       console.log("Error Downloading image"); 
       // propagate error after logging it 
       throw err; 
      }); 
     }); 
    }); 
    return Promise.all(promises); 
} 

オリジナル問題:

  1. は、ローカル、URLとして/
  2. 内の変数のための配列を繰り返すことはありません、idがで宣言されましたループが呼び出されるたびにこれらの値が破棄される可能性があります。
  3. エラーが正しくトップレベルに伝播されていませんでした(呼び出し元は、多少の誤差)

注:request.get().then()が自動的にあなたのための2xxの状態をチェックし、あなたが.then()を使用している場合、そのコードを削除してもらう代わりに開始されない場合拒否されます使用して、リクエスト-約束ライブラリで

  1. 、プレーンコールバック。
  2. 通常、エラー伝播が困難になるため、約束事とプレーンコールバックを混在させたくありません。そこで、私はfs.writeFile()という有名なバージョンを作成しました。 Bluebird約束ライブラリを使用している場合、モジュール全体を一度に約束することができます。
  3. 最初の配列から1対1の配列を生成するのは、.map()のように設計されています。また、関数閉包を提供するので、独自のIIFEを作成する必要はありません。
  4. ループ内の非同期コールバックで使用される変数は、ループの他の反復で上書きされないように、適切なスコープ(可能な限りローカル)にする必要があります。
+0

偉大な答えと追加のメモをありがとう!それは幾分構造化され始めたが、悪化し、さらに悪化した:)まだ多くのことを学ぶには、ありがとう! –

関連する問題