2
\$\begingroup\$

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.

\$\endgroup\$
1
\$\begingroup\$

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');
}
\$\endgroup\$

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.