0

I am building a small node.js website with a user interface that features a dropdown with a list of countries.

Previously the list of countries was hard coded in a json file that I would read:

exports.countries = require('./json/countries.json');

Then I realized I shouldn't hard code it like that when I can do a distinct query to the the list from the mongodb database.

db.collection.distinct('c', {}, function(err, data) {
  // something
});

But then there's the question of how to extract the value of the data variable in that callback function. I discovered that this works:

db.collection.distinct('c', {}, function(err, data) {
  if (err) {
    throw Error('mongodb problem - unable to load distinct values');
  } else {
    exports.countries = data;
  }
});

I am new to node.js and this seems fishy to me. Is this OK code? Is it better do this with generators or promises? If I wanted to use generators or promises to do this, how would I do that?

The end result where this is used is in a template. ref.countries is the actual list of countries using my fishy code. If I had a Promise instead of the list of countries, how would I change this code?

  <% ref.countries.forEach(function(c) { -%>
  <option value="<%= c %>">
    <%= ref.isoCodes[c] -%>
  </option>
  <% }); -%>

I am using node v6.10.3.

Jim
  • 1,740
  • 2
  • 13
  • 18
  • Do you expect the list of countries in the world to change while your app is running? Just cache the list in a file and read it once, when the app starts. – Traveling Tech Guy Jun 27 '17 at 22:46
  • *"If I wanted to use generators or promises to do this, how would I do that?"* You would export a function that returns a promise which resolves to the result of the DB query. What you have isn't useful because the code that loads the module won't know when `exports.countries` is available. I imagine that are a lot of tutorials on Promises on the interwebs. – Felix Kling Jun 27 '17 at 22:48
  • The list of countries will not change. Previously I was caching the list in a file but I don't want to do it that way. I want the code to query mongodb once at startup and use that list of countries. – Jim Jun 27 '17 at 22:49
  • How do I export a function that returns a promise which resolves the result of the DB query? I spent the past hour googling promises tutorials and didn't figure it out. I agree that the code that I have isn't useful because it doesn't know when `exports.countries` is available. That's why I thought this is fishy. – Jim Jun 27 '17 at 22:51
  • `exports.getCountries = function() { return new Promise((resolve, reject) => { db.collection.distinct(..., function(err, data) { /* reject on error */ resolve(data); }); }); };` – Felix Kling Jun 27 '17 at 22:52
  • What are `resolve` and `reject` supposed to be here? I don't want to call another function, I want the actual data. – Jim Jun 27 '17 at 22:54
  • This might help too: [How do I convert an existing callback API to promises?](https://stackoverflow.com/q/22519784/218196) – Felix Kling Jun 27 '17 at 22:54
  • `resolve` and `reject` are provided by the promises constructor for settling the promise. Maybe have a look at https://developers.google.com/web/fundamentals/getting-started/primers/promises to get more familiar with the basics of promises? *"I want the actual data"* The promise is resolved with the data. – Felix Kling Jun 27 '17 at 22:57
  • What does your startup script look like, and where does `db.collection.distinct..` live comparatively? It will help us provide a tangible answer to your question. – Jesse Kernaghan Jun 27 '17 at 22:57
  • This is in node.js code that I use elsewhere with `const reference = require('_/reference');`. Later `reference.countries` gets put into a dropdown with my template. – Jim Jun 27 '17 at 23:07
  • It doesn't seem I can do what I want with Promises. What about Generators? – Jim Jun 27 '17 at 23:08
  • Just updated the question to show where the list of countries are being used. – Jim Jun 27 '17 at 23:14

2 Answers2

0

Your export that you say "works" is impossible to use because the code that loads your module would have no idea when the exports.countries value has actually been set because it is set in an asynchronous call that finishes some indeterminate time in the future. In addition, you have no means of handling any error in that function.

The modern way of doing this would be to export a function that, when called, returns a promise that resolves to the data (or rejects with an error). The code loading your module, then calls that exported function, gets the promise, uses .then() on the promise and uses the data in the .then() handler. That could look something like this:

function getCountries() {
    return new Promise(function(resolve, reject) {
        db.collection.distinct('c', {}, function(err, data) {
          if (err) {
            reject(err);
          } else {
            resolve(data);
          }
        });
    }
}

module.exports.getCountries = getCountries;

The caller would then do something like this:

const myModule = require('myModule');
myModule.getCountries().then(function(countries) {
    console.log(countries);
    // use country data here
}).catch(function(err) {
    // deal with error here
});

Most databases for node.js these days have some level of promise support built in so you often don't have to create your own promise wrapper around your DB functions like was shown above, but rather can use a promise directly returned from the DB engine. How that works is specific to the particular database/version you are using.


If you are using the list of countries in a template rendering operation, then you will have to fetch the list of countries (and any other data needed for the template rendering) and only call res.render() when all the data has been successfully retrieved. This probably also leads to what you should do when there's an error retrieving the necessary data. In that case, you would typically respond with a 5xx error code for the page request and may want to render some sort of error page that informs the end-user about the error.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

I am using Node 6.10 so I don't have async and await but if I did they would help me here:

https://developers.google.com/web/fundamentals/getting-started/primers/async-functions

Instead I can use the asyncawait library:

https://github.com/yortus/asyncawait

Code looks like this:

var async = require('asyncawait/async');
var await = require('asyncawait/await');

const db = require('_/db');

function getDistinctValues(key) {
  return new Promise((resolve, reject) => {
    db.collection.distinct(key, {}, function(err, data) {
      if (err) {
        throw Error('mongodb problem - unable to load distinct values');
      } else {
        resolve(data);
      }
    });
  });
};

async(function () {
    exports.countries = await(getDistinctValues('c'));
    exports.categories = await(getDistinctValues('w'));
})();

Now I can be sure ref.countries and ref.categories are available after this is loaded.

Jim
  • 1,740
  • 2
  • 13
  • 18