node.js connecting to s3 and youtube

Question

I have a task to connect to S3 get a JSON file with the names of artists, use the youtube-data-api and search for the top 10 songs. Then send it back in a given format to S3 back.

console.log("Loading up the best code ever!!!");

//packagess import
var fs = require('fs');
var AWS = require('aws-sdk');
var Singer = require('./Singer')

//prepare all the AWS S3 data
AWS.config.update({ region: "us-west-1" });
var credentials = new AWS.SharedIniFileCredentials();
AWS.config.credentials = credentials;
// Create S3 service object
s3 = new AWS.S3({ apiVersion: '2006-03-01' });

// Create the parameters for calling createBucket
var bucketParams = {
    Bucket: 'pc-backend-exercises',
    Key: 'toSearch.json',
    ResponseContentType: 'application/json'
};

/// prepare the youtube-data-api
var YTAPI = require('node-youtubeapi-simplifier');
var APIKEY = 'AIzaSyDS1p3m9wnZLLCPc1hDQBX3K_UnS4j0CdY' 
YTAPI.setup(APIKEY);

console.log('connecting to s3 to get the json file');

var singers = [];

s3.getObject(bucketParams, function (err, data) {
    // Handle any error and exit
    if (err) {
        console.log(err, err.stack);
        return err;
    }
    var fileContents = data.Body.toString();
    var json = JSON.parse(fileContents);

    for (var i = 0; i < json.Search.artists.length; i++) {
        var newSinger = new Singer(json.Search.artists[i]);
        singers.push(newSinger);
    }
    console.log('the list of the singers has %d singers',json.Search.artists.length);
    search10TopForASinger();
});


//this function searches using youtube-api-simplfier for each artist name
//returns the list of songs and collects only 10, stores them into a map in the singer object
function search10TopForASinger() {
    console.log('entered search10TopForASinger ')
    var promises = [];

    for (var i = 0; i < singers.length; i++) {
        //Gets only 10 results
        promises.push(YTAPI.searchFunctions.simpleSearch(singers[i].name));
    }
    Promise.all(promises)
           .then((results) => {
               console.log('all the threads of the youtube api are back')
               for (var i = 0; i < singers.length; i++) {
                   setSongsArray(results[i], singers[i].songs);
               }

               printJsonAndUpload();
               console.log('we are done you can test yourself with testGiladFile.js');
            })
             .catch((e) => {
                    // Handle errors here
            });

}


//sets 10 or less songs for each artist into an array 
//we use the Singer object in order to store the data
function setSongsArray(data, songs) {
    console.log('entered setSongsArray')
    var size = 10;
    if (data.length < 10) {
        size = data.length;
    }

    for (var i = 0; i < size; i++) {
       songs.push(data[i].title);
    }
}

//The team wanted a json file with a certain format
//we create here to format and upload it back to S3
function printJsonAndUpload() {

    console.log('entered printJson')
    var data = {
        results: []
    };

    for (var i = 0; i < singers.length; i++) {
        var singerName = singers[i].name;
        var singerObj = {}
        singerObj[singerName] = []
        for (var song = 0; song < singers[i].songs.length; song++) {
            var songObj = {};
            songObj[song+1] = singers[i].songs[song];
            singerObj[singerName].push(songObj)
        }
        data.results.push(singerObj);
    }

    console.log('uploading file to s3')
    s3.putObject({Bucket: 'pc-backend-exercises',Key:'gilad.json',Body: JSON.stringify(data), ContentType: "application/json"},
        function(err, data) {
            //console.log('error');
            //console.log(JSON.stringify(err)+" "+JSON.stringify(data));
        });
    console.log('the name of the file is gilad.json');
}

This is the first code I have ever written in Node.js, please give me any feedback about style and performance.

Answer 1

I had a crack at it, I tried to replace the for loops with higher-order functions, and use ES6 let and const for variable declarations. I feel like the efficiency of the functions could be improved, but this could be a good starting point to refactoring. Also, fixed some formatting to make it a bit easier to read.

const AWS = require('aws-sdk');
const fs = require('fs');
const Singer = require('./Singer')
const YTAPI = require('node-youtubeapi-simplifier');

const credentials = new AWS.SharedIniFileCredentials();
const s3 = new AWS.S3({ apiVersion: '2006-03-01' }); //does s3 need to be declared as variable?

const APIKEY = 'AIzaSyDS1p3m9wnZLLCPc1hDQBX3K_UnS4j0CdY' 

const bucketParams = {
  Bucket: 'pc-backend-exercises',
  Key: 'toSearch.json',
  ResponseContentType: 'application/json'
};

AWS.config.update({ region: "us-west-1" });
AWS.config.credentials = credentials;
YTAPI.setup(APIKEY);

const singers = []; //do you need this singers array?

s3.getObject(bucketParams, (err, data) => {
    if (err) {
      console.log(err, err.stack);
      return err;
    }

    let fileContents = data.Body.toString();
    let json = JSON.parse(fileContents);
    //I try to avoid for loops if possible
    json.Search.artists.forEach((artist, idx) => singers.push(new Singer(json.Search.artists[idx])));

    console.log('the list of the singers has %d singers',json.Search.artists.length);
    search10TopForASinger();
});

const setSongsArray = (data, songs) => {
  data.slice(0, 10).forEach((song) => songs.push(song.title));
}

const search10TopForASinger = () => {
  console.log('entered search10TopForASinger ')

  let promises = singers.map((singer, idx) => YTAPI.searchFunctions.simpleSearch(singers[idx].name));

  Promise.all(promises).then((results) => {
    console.log('all the threads of the youtube api are back')
    singers.forEach((singer, idx) => setSongsArray(results[idx], singers[idx].songs) )
    printJsonAndUpload();
    console.log('we are done you can test yourself with testGiladFile.js');
  }).catch((e) => {
    console.log(e)
  });
}

const printJsonAndUpload = () => {
  console.log('entered printJson')
  const data = {
    results: []
  };

  singers.forEach((singer, idx) => {
    let singerName = singers[idx].name;
    const singerObj = {};
    singerObj[singerName] = [];
    singers[idx].songs.forEach((song) => { //could use .reduce() here
      const songObj = {};
      songObj[song+1] = singers[i].songs[song];
      singerObj[singerName].push(songObj)
    });
    data.results.push(singerObj);
  });

  console.log('uploading file to s3')
  s3.putObject({ Bucket: 'pc-backend-exercises', Key:'gilad.json', Body: JSON.stringify(data), ContentType: "application/json" },
    (err, data) => {
      //console.log('error');
      //console.log(JSON.stringify(err)+" "+JSON.stringify(data));
    });
  console.log('the name of the file is gilad.json');
}