2

私はJsonRestStoresの作者です。私はこの問題を少し長らく延期してきました。これは、「今年最も愚かにインデントされた関数」賞を受賞する関数です。

主な厄介な問題は、クロージャー変数が変更されるポイントがあることです。

body[ self.idProperty ] = params[ self.idProperty ];

物事を面白くする「if」もあります。

それで...この関数を、2つの突きのある矢のように見えないようにするエレガントな方法はありますか? もしそうなら、実装例を提供できますか?

  _makePostAppend: function( params, body, options, next ){

    var self = this;
    var body;

    if( typeof( next ) !== 'function' ) next = function(){};

    // Check that the method is implemented
    if( ! self.handlePostAppend ){
      self._sendError( next, new self.NotImplementedError( ) );
      return;
    }

    // Check the IDs
    self._checkParamIds( params, body, false, function( err ){  
      self._sendErrorOnErr( err, next, function(){

        self.schema.validate(  body, function( err, body, errors ) {
          self._sendErrorOnErr( err, next, function(){

            if( errors.length ){
              self._sendError( next, new self.UnprocessableEntityError( { errors: errors } ) );
            } else {

              // Fetch the doc
              self.execAllDbFetch( params, body, options, function( err, fullDoc ){
                self._sendErrorOnErr( err, next, function(){


                  self.extrapolateDoc( params, body, options, fullDoc, function( err, doc) {
                    self._sendErrorOnErr( err, next, function(){

                      self._castDoc( doc, function( err, doc) {
                        self._sendErrorOnErr( err, next, function(){

                          // Actually check permissions
                          self.checkPermissionsPostAppend( params, body, options, doc, fullDoc, function( err, granted ){
                            self._sendErrorOnErr( err, next, function(){

                              if( ! granted ){
                                self._sendError( next, new self.ForbiddenError() );
                              } else {

                                // Clean up body from things that are not to be submitted
                                //if( self.schema ) self.schema.cleanup( body, 'doNotSave' );
                                self.schema.cleanup( body, 'doNotSave' );

                                // Paranoid check
                                // Make sure that the id property in the body does match
                                // the one passed as last parameter in the list of IDs
                                body[ self.idProperty ] = params[ self.idProperty ];

                                self.execPostDbAppend( params, body, options, doc, fullDoc, function( err, fullDocAfter ){
                                  self._sendErrorOnErr( err, next, function(){

                                    self.extrapolateDoc( params, body, options, fullDocAfter, function( err, doc) {
                                      self._sendErrorOnErr( err, next, function(){

                                        self._castDoc( fullDocAfter, function( err, docAfter) {
                                          self._sendErrorOnErr( err, next, function(){

                                            // Remote request: set headers, and send the doc back (if echo is on)
                                            if( self.remote ){
                                              if( self.echoAfterPostAppend ){

                                                 self.prepareBeforeSend( docAfter, function( err, docAfter ){
                                                   self._sendErrorOnErr( err, next, function(){

                                                      self.afterPostAppend( params, body, options, doc, fullDoc, docAfter, fullDocAfter, function( err ){
                                                        self._sendErrorOnErr( err, next, function(){

                                                          self._res.json( 200, docAfter );

                                                        });
                                                      });
                                                   })
                                                 })
                                              } else { 

                                                self.afterPostAppend( params, body, options, doc, fullDoc, docAfter, fullDocAfter, function( err ){
                                                  self._sendErrorOnErr( err, next, function(){

                                                    self._res.send( 204, '' );

                                                  });
                                                });

                                              }

                                            // Local request: simply return the doc to the asking function
                                            } else {

                                              self.prepareBeforeSend( docAfter, function( err, docAfter ){
                                                self._sendErrorOnErr( err, next, function(){

                                                  self.afterPostAppend( params, body, options, doc, fullDoc, docAfter, fullDocAfter, function( err ){
                                                    self._sendErrorOnErr( err, next, function(){

                                                      next( null, docAfter, self.idProperty );

                                                    })
                                                  })

                                                })
                                              })
                                            }

                                          })
                                        });

                                      });
                                    })


                                  }) // err
                                }) // execPostDbAppend

                              } // granted


                            })
                          }) 

                        }) 
                      }) 

                    })
                  }) 

                }) // err
              }) // checkPermissionsPostAppend

            } // errors.length

          }) // err
        }) // self.validate

      }) // err
    }) // self.validate
  },
4

4 に答える 4

2

Promise を使用すると、例外のバブリングと戻り値の構成が復元されるため、非同期コードを同期コードからほぼ直接記述できます。

他のメソッドを既に promise に書き換えていると仮定します。

var Promise = require("bluebird");

...

_makePostAppend: function (params, body, options) {
  var fullDoc, doc, docAfter, fullDocAfter;
  // Check that the method is implemented
  if (!this.handlePostAppend) {
    return Promise.rejected(new this.NotImplementedError());
  }

  //Note that it is Promise#bind, not Function#bind
  return this._checkParamIds(param, body, false).bind(this).then(function () {
    return this.schema.validate(body);
  }).then(function () {
    return this.execAllDbFetch(params, body, options);
  }).then(function (_fullDoc) {
    fullDoc = _fullDoc;
    return this.extrapolateDoc(params, body, options, fullDoc);
  }).then(function (doc) {
    return this._castDoc(doc);
  }).then(function (_doc) {
    doc = _doc;
    return this.checkPermissionsPostAppend(params, body, options, doc, fullDoc);
  }).then(function (granted) {
    if (!granted) throw new this.ForbiddenError();

    this.schema.cleanup(body, 'doNotSave');
    body[this.idProperty] = params[this.idProperty];
    return this.execPostDbAppend(params, body, options, doc, fullDoc);
  }).then(function (_fullDocAfter) {
    fullDocAfter = _fullDocAfter;
    return this.extrapolateDoc(params, body, options, fullDocAfter);
  }).then(function (doc) {
    return this._castDoc(fullDoc);
  }).then(function (_docAfter) {
    docAfter = _docAfter;
    if (this.remote) {
      if (this.echoAfterPostAppend) {
        return this.prepareBeforeSend(docAfter).bind(this).then(function (_docAfter) {
          docAfter = _docAfter;
          return this.afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter);
        }).then(function () {
          return this._res.json(200, docAfter);
        });
      } else {
        return this.afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter).bind(this).then(function () {
          return this._res.send(204, '');
        });
      }
    } else {
      return this.prepareBeforeSend(docAfter).then(function (_docAfter) {
        docAfter = _docAfter;
        return this.afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter);
      });
    }
  });
}

2 スペースのインデントでカンニングする必要がなくなったことに注意してください。上記は 4 スペースのインデントでより読みやすくなります。多分それは私だけです。

使用法は次のとおりです。

this._makePostAppend(params, body, options).bind(this).then(function() {

}).catch(this.UnprocessableEntityError, function(e) {

}).catch(this.NotImplementedError, function(e) {

}).catch(this.ForbiddenError, function(e) {

}).catch(function(e) {
  //Any other error
});
于 2013-11-09T15:06:09.677 に答える
2

あなたのようなコードを書いていたら、非同期ライブラリを使用したいと思います。これはウォーターフォール関数であるため、非同期 API を promise バージョンでラップする必要はありません。それは非常に簡単です。約束も素晴らしく、@Esailjaの答えには何も問題はありませんが、個人的には、これは実装がはるかに簡単で読みやすいと思います:

var async = require('async');

var _makePostAppend = function (params, body, options, next) {
  var self = this, body;
  if (typeof(next) !== 'function') next = function () { };

  // Check that the method is implemented
  if (!self.handlePostAppend) {
    self._sendError(next, new self.NotImplementedError());
    return;
  }

  async.waterfall([
    function (cb) {
      // Check the IDs
      self.checkParamIds(params, body, false, cb);
    },
    function (cb) {
      self.schema.validate(body, cb);
    },
    function (body, errors, cb) {
      if (errors.length) cb(new self.UnprocessableEntityError({ errors: errors }));
      // Fetch the doc
      self.execAllDbFetch(params, body, options, cb);
    },
    function (fullDoc, cb) {
      self.extrapolateDoc(params, body, options, fullDoc, function (err, doc) {
        cb(err, fullDoc, doc);
      });
    },
    function (fullDoc, doc, cb) {
      self._castDoc(doc, function (err, doc) {
        cb(err, fullDoc, doc);
      });
    },
    function (fullDoc, doc, cb) {
      // Actually check permissions
      self.checkPermissionsPostAppend(params, body, options, doc, fullDoc, function (err, granted) {
        cb(err, fullDoc, doc, granted);
      });
    },
    function (fullDoc, doc, granted, cb) {
      if (!granted) cb(new self.ForbiddenError());

      // Clean up body from things that are not to be submitted
      //if( self.schema ) self.schema.cleanup( body, 'doNotSave' );
      self.schema.cleanup(body, 'doNotSave');

      // Paranoid check
      // Make sure that the id property in the body does match
      // the one passed as last parameter in the list of IDs
      body[self.idProperty] = params[self.idProperty];

      self.execPostDbAppend(params, body, options, doc, fullDoc, function (err, fullDocAfter) {
        cb(err, fullDoc, fullDocAfter);
      });
    },
    function (fullDoc, fullDocAfter, cb) {
      self.extrapolateDoc(params, body, options, fullDocAfter, function (err, doc) {
        cb(err, fullDoc, doc, fullDocAfter);
      });
    },
    function (fullDoc, doc, fullDocAfter, cb) {
      self._castDoc(fullDocAfter, function (err, docAfter) {
        cb(err, fullDoc, doc, fullDocAfter, docAfter);
      });
    }
  ], function (err, fullDoc, doc, fullDocAfter, docAfter) {
    self._sendErrorOnErr(err, next, function () {
      // Remote request: set headers, and send the doc back (if echo is on)
      if (self.remote) {
        if (self.echoAfterPostAppend) {
          async.waterfall([
            function (cb) {
              self.prepareBeforeSend(docAfter, cb);
            },
            function (docAfter, cb) {
              self.afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter, cb)
            }
          ], function (err, docAfter) {
            self._sendErrorOnErr(err, next, function () {
                self._res.json(200, docAfter);
            });
          });
        } else {
          self.afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter, function (err) {
            self._sendErrorOnErr(err, next, function () {
              self._res.send(204, '');
            });
          });
        }

        // Local request: simply return the doc to the asking function
      } else {
        async.waterfall([
          function (cb) {
            self.prepareBeforeSend(docAfter, function (err, docAfter) {
              cb(err, doc, fullDoc, fullDocAfter, docAfter);
            })
          },
          function (doc, fullDoc, fullDocAfter, docAfter, cb) {
            self.afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter, function (err) {
              cb(err, docAfter);
            });
          }
        ], function (err, docAfter) {
          self._sendErrorOnErr(err, next, function () {
            next(null, docAfter, self.idProperty);
          });
        });
      }
    });
  });
};

またはさらに良いことに、@Esailja の回答のスコーピング トリックを使用しました。

var async = require('async');

var _makePostAppend = function (params, body, options, next) {
  var _self = this, _body, _fullDoc, _doc, _docAfter, _fullDocAfter;
  if (typeof(next) !== 'function') next = function () { };

  // Check that the method is implemented
  if (!_self.handlePostAppend) {
    _self._sendError(next, new _self.NotImplementedError());
    return;
  }

  async.waterfall([
    function (cb) {
      // Check the IDs
      _self.checkParamIds(params, _body, false, cb);
    },
    function (cb) {
      _self.schema.validate(_body, cb);
    },
    function (body, errors, cb) {
      if (errors.length) cb(new _self.UnprocessableEntityError({ errors: errors }));
      // Fetch the doc
      _self.execAllDbFetch(params, body, options, cb);
    },
    function (fullDoc, cb) {
      _fullDoc = fullDoc;
      _self.extrapolateDoc(params, _body, options, fullDoc, db);
    },
    function (doc, cb) {
      _self._castDoc(doc, cb);
    },
    function (doc, cb) {
      _doc = doc;
      // Actually check permissions
      _self.checkPermissionsPostAppend(params, _body, options, doc, _fullDoc, cb);
    },
    function (granted, cb) {
      if (!granted) cb(new _self.ForbiddenError());

      // Clean up body from things that are not to be submitted
      //if( self.schema ) self.schema.cleanup( body, 'doNotSave' );
      _self.schema.cleanup(_body, 'doNotSave');

      // Paranoid check
      // Make sure that the id property in the body does match
      // the one passed as last parameter in the list of IDs
      _body[_self.idProperty] = params[_self.idProperty];

      _self.execPostDbAppend(params, _body, options, _doc, _fullDoc, cb);
    },
    function (fullDocAfter, cb) {
      _fullDocAfter = fullDocAfter;
      _self.extrapolateDoc(params, _body, options, fullDocAfter, cb);
    },
    function (doc, cb) {
      _doc = doc;
      _self._castDoc(_fullDocAfter, cb);
    }
  ], function (err, docAfter) {
    _self._sendErrorOnErr(err, next, function () {
      // Remote request: set headers, and send the doc back (if echo is on)
      if (_self.remote) {
        if (_self.echoAfterPostAppend) {
          async.waterfall([
            function (cb) {
              _self.prepareBeforeSend(docAfter, cb);
            },
            function (docAfter, cb) {
              _self.afterPostAppend(params, _body, options, _doc, _fullDoc, docAfter, _fullDocAfter, cb)
            },
            function (cb) {
              _self._res.json(200, docAfter);
              cb();
            }
          ], function (err, results) {
            _self._sendErrorOnErr(err, next);
          });
        } else {
          _self.afterPostAppend(params, _body, options, _doc, _fullDoc, docAfter, _fullDocAfter, function (err) {
            _self._sendErrorOnErr(err, next, function () {
              _self._res.send(204, '');
            });
          });
        }

        // Local request: simply return the doc to the asking function
      } else {
        async.waterfall([
          function (cb) {
            _self.prepareBeforeSend(docAfter, cb);
          },
          function (docAfter, cb) {
            _docAfter = docAfter;
            _self.afterPostAppend(params, _body, options, _doc, _fullDoc, docAfter, _fullDocAfter, cb);
          }
        ], function (err) {
          _self._sendErrorOnErr(err, next, function () {
            next(null, _docAfter, _self.idProperty);
          });
        });
      }
    });
  });
};
于 2013-11-09T21:10:06.017 に答える
1

コールバック地獄は怖いところです:)

コールバックをより読みやすい一連のステップにするStep.jsのようなものを使用できます。他にも多くの非同期管理ライブラリがありますが、それが本当にここであなたを救うかどうかはわかりません...あなたはまだごちゃごちゃした混乱を抱えています.それほど大きくインデントされていないだけです.


手続き的に考えるのをやめて、データ モデル、どのオブジェクトがどのメソッドを持っているか、どのメソッドがどのアトミック タスクを担当しているかについて考え始めることをお勧めします。

したがって、過度に肥大化したメソッドと同じ方法で単純にリファクタリングします。つまり、関連するコードのビットを独自のメソッドに抽象化します。

self.checkIds(function() {
  self.fetchDoc(function() {
    self.checkPermissions({
      deny: self.denyPermission,
      allow: function() {
        // call method that handles the next thing
      }
    })
  })
});

これで、実際の作業が行われるコンポーネント メソッドを呼び出すだけのマクロ メソッドができました。これらの各メソッドは、内部的にいくつかのコールバックを持ち、「すべて完了」したコールバックを呼び出して、制御をマクロ関数に戻します。

これには、実際に何が起こっているかに基づいて、この多数のステップを読み取って理解できるという追加の利点があります。それは今ではステップバイステップの指示のように読めますが、代わりに低レベルのノイズの巨大な山です.

そのため、非常に単純な非同期処理の小さなセットを実行し、完了したらコールバックする、はるかに小さなメソッドを 12 個作成します。そして、必ずわかりやすい名前を付けてください。次に、それらをすべてまとめて、より小さくて保守しやすいコールバックのツリーにリンクします。

この大きなメソッドがコメントなしで意味をなすことができれば、はるかに優れたバージョンにかなり近いことがわかります。

頑張ってください、あなたはこれであなたの仕事を切り取りました.


いくつかの一般的なパターンもあることに注意してください。

ここみたいに:

self.extrapolateDoc( params, body, options, fullDocAfter, function( err, doc) {
  self._sendErrorOnErr( err, next, function(){

そしてここ:

self.afterPostAppend( params, body, options, doc, fullDoc, docAfter, fullDocAfter, function( err ){
  self._sendErrorOnErr( err, next, function(){

メソッド名と引数リストを受け取りself._sendErrorOnErr()、最初のメソッドがコールバックしたときに自動的に処理を進めるメソッドを作成できます。

より一般的なパターンを見つけると、これをさらに削減できます。

于 2013-11-09T11:11:14.913 に答える
-2

これは少しごまかしのない答えですが、うまくいけば、少なくとも少しは明るくなります。最初に行うことは、コード自体の同期バージョンを単純化することです。次のようなものから始めます。

_makePostAppend: function (params, body, options) {
  // ...and in the darkness, bind them (if needed)
  _.bindAll(this, 'checkParamIds', 'execAllDbFetch'); // ...etc.
  // Shortcuts. These would be unnecessary for private methods in a closure
  var
    checkParamIds = this.checkParamIds,
    schema = this.schema,
    execAllDbFetch = this.execAllDbFetch,
    execPostDbAppend = this.execPostDbAppend,
    extrapolateDoc = this.extrapolateDoc,
    checkPermissionsPostAppend = this.checkPermissionsPostAppend,
    _castDoc = this._castDoc,
    UnprocessableEntityError = this.UnprocessableEntityError,
    ForbiddenError = this.ForbiddenError,
    idProperty = this.idProperty,
    remote = this.remote,
    echoAfterPostAppend = this.echoAfterPostAppend,
    prepareBeforeSend = this.prepareBeforeSend,
    afterPostAppend = this.afterPostAppend,
    _res = this._res;

  checkParamIds(params, body, false);

  // This could throw the UnprocessableEntityError itself
  var errors = schema.validate(body);
  if (errors.length) {
    throw new UnprocessableEntityError({ errors: errors });
  }

  // Every method takes params+body+options. Wrap that in a single object? Alternatively:
  // var processor = this.getDocProcessor(params, body, options);
  // var fullDoc = processor.execAllDbFetch();
  // var doc = extrapolateDoc(fullDoc);
  // ...etc.
  var fullDoc = execAllDbFetch(params, body, options);
  var doc = extrapolateDoc(params, body, options, fullDoc);
  doc = _castDoc(doc);

  // This could throw the ForbiddenException itself
  var granted = checkPermissionsPostAppend(params, body, options, doc, fullDoc);
  if (!granted) { throw new ForbiddenError(); }

  schema.cleanup(body, 'doNotSave');
  body[idProperty] = params[idProperty];

  var fullDocAfter =  execPostDbAppend(params, body, options, doc, fullDoc);
  var docAfter =  extrapolateDoc(params, body, options, fullDocAfter);
  docAfter = _castDoc(docAfter);

  if (!remote || echoAfterPostAppend) {
    docAfter = prepareBeforeSend(docAfter);
  }
  afterPostAppend(params, body, options, doc, fullDoc, docAfter, fullDocAfter);
  return remote ?
    _res.json(200, echoAfterPostAppend ? docAfter : '') :
    docAfter;
}

さて、ジェネレーターがある場合は、非同期メソッドの前にyieldを付けるだけで、残りを実行するライブラリー関数に渡すことができます(Q.star、1つだと思います)。非同期メソッドを約束するか、少なくとも入力をバインドする必要があります (つまり、func(in1, in2) が apply(func, in1, in2) になります)。単純な古い同期コード。

ただし、ジェネレーターを持っていない可能性があるため、チートの少ない答えも用意してください。チートが少ないということは、上記のメモで説明されているように、少し単純化されたバリアントから始めていることを意味します。つまり、同期コードは次のようになります。

// Helper method. Could be private, defined elsewhere, etc.
var extrapolateCast = function (fullDoc) {
  var doc = extrapolateDoc(fullDoc);
  return _castDoc(doc);
};

_makePostAppend: function (params, body, options) {
  checkParamIds(params, body, false);
  schema.validate(body);
  var proc = getDocProcessor(params, body, options);

  var fullDoc = proc.execAllDbFetch();
  var doc = extrapolateCast(fullDoc);

  proc.checkPermissionsPostAppend(doc, fullDoc);

  schema.cleanup(body, 'doNotSave');
  body[idProperty] = params[idProperty];

  var fullDocAfter =  proc.execPostDbAppend(doc, fullDoc);
  var docAfter =  extrapolateCast(fullDocAfter);

  if (!remote || echoAfterPostAppend) {
    docAfter = prepareBeforeSend(docAfter);
  }
  proc.afterPostAppend(doc, fullDoc, docAfter, fullDocAfter);
  return remote ?
    _res.json(200, echoAfterPostAppend ? docAfter : '') :
    docAfter;
}

20 行程度で、はるかに扱いやすくなります。非同期では、コンテキスト オブジェクトを使用する 1 つのソリューションは次のようになります。(はい、これは多くの点でひどいことを知っていますが、すでにこれに多くの時間を費やしてきました。)編集:これは実際には機能しません。コンテキストからの変数は、実行直前ではなく、すぐに解決されます。 . それらを関数でラップして遅延させるか、「保存」に類似した「ロード」関数を実装できます。

_makePostAppend: function (params, body, options) {
  // Helper method. Could be private, defined elsewhere, etc.
  var extrapolateCast = pipeline(
    extrapolateDoc,
    _castDoc
  );

  var vars = {},
    contextObj = context(vars),
    save = contextObj.save,
    record = contextObj.record,
    proc = getDocProcessor(params, body, options);

  pipeline(
    apply(checkParamIds, params, body, false),
    apply(schema.validate, body),
    record('fullDoc', proc.execAllDbFetch),
    save('doc', extrapolateCast),
    apply(proc.checkPermissionsPostAppend, vars.doc, vars.fullDoc),
    apply(schema.cleanup, body, 'doNotSave'),
    lift(function () { body[idProperty] = params[idProperty]; }),
    record('fullDocAfter', apply(proc.execPostDbAppend, vars.doc, vars.fullDoc)),
    record('docAfter', extrapolateCast),
    conditional(
      function () { return !remote || echoAfterPostAppend; },
      prepareBeforeSend
    ),
    apply(proc.afterPostAppend, vars.doc, vars.fullDoc, vars.docAfter, vars.fullDocAfter)
  );

  // NOTE: Converting this part is left as an exercise for the reader. (read: I'm tired.)
  return remote ?
    _res.json(200, echoAfterPostAppend ? docAfter : '') :
    docAfter;
};

以下に表示されるヘルパー メソッド:

var 
context = function (store) {
  return {
    save: function (key, task) {
      return pipeline(
        task,
        tap(lift(function (result) {
          store[key] = result;
        }))
      );
    },
    // Same as save, but discards result
    record: function (key, task) {
      return pipeline(
        task,
        liftM(function (result) {
          store[key] = result;
          return [];
        })
      );
    }
  };
},
conditional = function (cond, task) {
  // NOTE: Too tired to figure this out now. Sorry!
},
createTask = function (impl) {
  return function () {
    var args = _.initial(arguments),
      callback = _.last(arguments);
    async.nextTick(function () {
      impl.call(null, args, callback);
    });
  };
},
liftM = function (fun) {
  return createTask(function (args, callback) {
    try {
      var results = fun.apply(this, args);
      return callback.apply(this, [null].concat(results));
    } catch (err) {
      return callback.call(this, err);
    }
  });
},
lift = function (fun) {
  return createTask(function (args, callback) {
    try {
      var result = fun.apply(this, args);
      return callback(null, result);
    } catch (err) {
      return callback(err);
    }
  });
},
tap = function (interceptor) {
  return createTask(function (args, callback) {
    return async.waterfall([
      _.partialArr(interceptor, args),
      liftM(function () {
        return args;
      })
    ], callback);
  });
};
于 2013-11-16T20:15:08.873 に答える