Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

app.js

var employees = require('../models/employees');

employees.read(req.params.id, function(body) {
    console.log(body.firstName);
});

models/employees

var request = require('request');

var employees = {

    read: function(id, callback) {
        request
            .get('http://api.mysite.com/employees/' + id, function(error, response, body) {
                body = JSON.parse(body);
                return callback(body);
            })
    },
};

module.exports = employees;

This works (returns the employee name correctly), but I'm not sure if this is the correct (async) way of getting data from an API and displaying it.

share|improve this question

You didn't implement any error handling. Before doing something with the api response body, you should check if an error occured. Also the callback function of your read method should use the error first pattern. Here is a very short explanation: http://www.codingdefined.com/2015/10/what-are-error-first-callbacks-in-nodejs.html

Further, JSON.parse can throw an error if your api doesn't respond with a valid json string. So you might want to handle this error, too. Just add a try-catch block.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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