6
\$\begingroup\$

I'm relatively new to NodeJS and Express, and I've recently run into the pyramid of Doom while doing stuff that requires multiple steps.

I've found that promises are very promising indeed. But I definitely need opinions on various aspects of my approach to routes as a whole.

Here's the full code:

I'm not very sure about this constructor whose only responsibility is basically to hold the promised steps and pipe the same input regardless of the results.

function SignupDoctorPostRequest(data) {
    this.validatePlanId = function() {
        return SubscriptionService.hasValidPlan(data.plan);
    };

    this.createUser = function () {
        return UserService.save(data);
    };

    this.subscribeUser = function () {
        return SubscriptionService.create(data);
    };
}

My intent is revealed when we look at the route, where we see the data is only the body of the request.

router.post('/', function (rq, response) {
    var validation = SignupDoctorPostSchema.validate(rq.body); // This is a Joi Schema validation.

    if(validation.error) {
        response.
          status(422).
          json({
            status: 'fail',
            data: joiful.parseValidationError(validation.error) // this is only method I use to parse joi validations into a more consistent object.
          });
        return;
    }

    var request = new SignupDoctorPostRequest(validation.value);

    request.validatePlanId()
    .then(request.createUser)
    .then(request.subscribeUser)
    .then(
        function Success (authenticatedUser) {
          response.status(201).json({});
        }
    )
    .catch(
        function Fail (fail) {
          var statusCode = fail.type === 'Validation' ? 422 : 500;
          var responseStatus = fail.type === 'Validation' ? 'fail' : 'error';
          var responseData = fail.type === 'Validation' ? fail.details : fail.message;

          response.
            status(statusCode).
            json({
              status: responseStatus,
              data: responseData
            });
        }
    );
});

Lastly, I need feedback on handling my responses because I'm responding eagerly if validation fail and then again if the the process fails, I think this is sort of smelly.

\$\endgroup\$
5
\$\begingroup\$

Interesting,

I actually really like your constructor, it is neat and shows in one place what the class does. I am not sure I would call it SignupDoctorPostRequest, since it is not a request at all but processes a request. I would consider DoctorSignupModel or since it seems to leverage several models DoctorSignupMetaModel.

As for your eager validation, I have to agree that it does not look very elegant. If you could work SignupDoctorPostSchema.validate( into this.validatePlanId (possibly call it then this.validate), you could rewrite your write to this:

router.post('/', function (rq, response) {

    var metaModel = new DoctorSignupMetaModel(rq.body);

    request.validate() //<- This will do all validation now
    .then(request.createUser)
    .then(request.subscribeUser)
    .then(
        function Success (authenticatedUser) {
          response.status(201).json({});
        }
    )
    .catch(
        function Fail (fail) {
          var statusCode = fail.type === 'Validation' ? 422 : 500;
          var responseStatus = fail.type === 'Validation' ? 'fail' : 'error';
          var responseData = fail.type === 'Validation' ? fail.details : fail.message;

          response.
            status(statusCode).
            json({
              status: responseStatus,
              data: responseData
            });
        }
    );
});
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I see your point with the validation. I think that's exactly were it belongs. As for the constructor, I was afraid I may need to create different ones for different type of requests (put maybe). But with the way you put it, what I can do is simply add methods as needed and use them accordingly on each type of request. Thanks for the review, much appreciated! \$\endgroup\$ – jhenriquez Dec 1 '14 at 18:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.