CODEWITHSUNDEEP
javascriptnode.js
reticentroot
reticentroot

Reputation: 3682

My solution seems "Ugly"

After a lot of tinkering and searching i am able to get this code tonwork. However it feels dirty and I had to go through a lot of trouble because node doesn't like to pass data to global variables, is there a better way to write this? I am very new to node so I feel like I missing pieces that would have made my life much easier. As per @JordanHendrix suggestion I will be moving over to template literals.

var wikipedia = require("node-wikipedia");
var infobox = require('wiki-infobox');
var Promise = require('es6-promise').Promise;

var fs = require('fs');

String.prototype.format = function()
{
   var content = this;
   for (var i=0; i < arguments.length; i++)
   {
        var replacement = '{' + i + '}';
        content = content.replace(replacement, arguments[i]);
   }
   return content;
};


function getLinks() {
  return new Promise(function(resolve, reject) {
    wikipedia.page.data("List_of_hip_hop_musicians", { content: true }, function(response) {
      fs.writeFile("response.json", JSON.stringify(response, null, 2), function(err) {
        if(err) {
          return console.log(err);
        }
      });
      resolve(response.links);
    });
  });
}


function getInfo(links){
  return new Promise(function(resolve, reject) {
    fs.appendFile("info.csv", "name, background, birth_name, birth_date, birth_place, origin, genre, occupation, instrument, years_active, label, website\n", function(err) {
      if(err) {
        return console.log(err);
      }
    });
    for (i=0; i<10; i++) {
      var link = links[i]['*'];
      resolveInfo(link).then(function(newText){
        fs.appendFile("info.csv", newText, function(err) {
          if(err) {
            return console.log(err);
          }
        });
      });
    }
    resolve(text)
  });
}


function resolveInfo(link) {
  return new Promise(function(resolve, reject) {
    infobox(link, 'en', function(err, data){
      if (err) {
        return console.log(err);
      }
      try {
        var text = '{0}, {1}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}, {11}\n'.format(data.name.value, data.background.value, data.birth_name.value, data.birth_date.value, JSON.stringify(data.birth_place), data.origin.value, data.genre.text, JSON.stringify(data.occupation), data.instrument.text, data.years_active.value, data.label.value, data.website.value);
      } catch(err){
        var text = "";
      }
      resolve(text);
    });
  });
}


getLinks().then(function(links) {
  getInfo(links).then(function(text){
  });
});

Upvotes: 0

Views: 140

Answers (1)

Johannes Merz
Johannes Merz

Reputation: 3342

I would consider a few things. First: use arrow functions whenever possible, makes you code to look much cleaner.

new Promise((resolve, reject) => {
  //...
});

You might wanna have a look into promisification to further cleanup. You would need to use bluebirds promise library though (which i'd recommend anyway).

var fs = require("fs");
Promise.promisifyAll(fs);
fs.readFileAsync("file.js", "utf8").then(...)

Last but not least, chain your promises properly:

getLinks()
  .then(getInfo)
  .then(text => console.log(text));

Upvotes: 1

Related Questions