Video downloader app implemented in RxJs

Question

I have used multiple map/flatmap combinations. Could anybody help me with the review?

var Rx = require('rx');
var RxNode = require('rx-node');
var request = require('request');
var _ = require('lodash');
var cheerio = require('cheerio');
exports.getResult = function(req, res) {
 var result = '';
 fetchContent({
 url: req.body.url
 }, req.body.season)
 .flatMap(getEpisodeRange)
 .map(formEpisodeUrl.bind(null, req.body.url, req.body.season))
 .flatMap(downloadHtml)
 .map(getGorillaUrl)
 .flatMap(postToGorilla)
 .map(getVideoUrl)
 .flatMap(prepareHtml)
 .subscribe(function(data) {
 result += data;
 }, function(error) {
 res.send(JSON.stringify(error))
 }, function() {
 res.send(result);
 });
}
function fetchContent(params) {
 console.log(params.url);
 var args = [].slice.call(arguments, 1)
 return Rx.Observable.create(function(observer) {
 request(params, function(error, response, body) {
 if (error) {
 observer.onError();
 } else {
 observer.onNext({
 response: response,
 body: body,
 args: args
 });
 }
 observer.onCompleted();
 })
 });
};
function prepareHtml(url) {
 return `<a href=${url}></a>`;
}
function fileName(url, season, episode) {
 return Rx.Observable.just(url.split('/'))
 .last().map(function(name) {
 return name.replace('_', ' ').concat(` S${season}E${episode}`);
 });
}
function getVideoUrl(data) {
 var pattern = /"http(.*)(\.flv|\.mkv|\.mp4)"/;
 var matches = pattern.exec(data.body);
 if (matches && matches[0]) {
 return matches[0]
 } else {
 return null;
 }
}
function postToGorilla(url) {
 var params = {
 url: url,
 method: 'POST',
 headers: {
 'content-type': 'application/x-www-form-urlencoded',
 },
 form: {
 op: 'download1',
 method_free: 'Free+Download',
 id: _.last(url.split('/')),
 }
 }
 return fetchContent(params)
}
function getGorillaUrl(data) {
 try {
 var $ = cheerio.load(data.body);
 var url = $('a[title="gorillavid.in"]').first().attr('href').split('=')[1]
 return url = new Buffer(url, 'base64').toString('ascii');
 } catch (error) {
 return Rx.Observable.just(null);
 }
}
function downloadHtml(url) {
 return fetchContent({
 url
 });
}
function formEpisodeUrl(url, season, episode) {
 return url.replace('/serie/', '/episode/').concat(`_s${season}_e${episode}.html`);
}
function getEpisodeRange(data) {
 var $ = cheerio.load(data.body);
 return _.range(1, ($('span').filter(function() {
 return $(this).text() === `Season ${data.args[0]}`;
 }).parents('h2').siblings('ul').children('li').length) + 1 || 0, 1);
}

• Dan: I have rolled back the last edit. Please see What to do when someone answers .

Answer 1

Overall, I think this is very decent code, some observations:

JSHint

• Use JsHint ;)
• You have a number of missing semicolons, and an unnecessary semicolon
• return url = new Buffer(url, 'base64').toString('ascii'); <- I dont think you wanted to assign to buffer?
• You are not using the function fileName(url, season, episode){ anywhere

Odd stuff

• You use the extremely cool fetchContent({ url }); but also the mundane

  {
   response: response,
   body: body,
   args: args
  }
  

  You should stick to 1 approach

• Your regex /"http(.*)(\.flv|\.mkv|\.mp4)"/
  • Is not supporting avi, wmv, or mpg file extensions.
  • Is not supporting uppercase incoming file names extentions.
  • Keeps repeating the \. for each extension, so I would keep that out of the brackets.
  • If you plan for many entries, then I would keep the regex outside of the function, and generate that regex only 1 single time for performance
• You have a stray console.log(params.url);, get rid of it

• It seemed odd that the HTML you prepare has no text in the anchor:

  return `<a href=${url}></a>`;
  

  Also, do you want to consider at least surrounding the URL with double quotes?

Sugar

• You use req.body 3 times in getResult, I would have had a var params = req.body; as a shortcut

• This seems a good use case for a ternary:

  if (matches && matches[0]) {
   return 
  } else {
   return null;
  }
  

  could be

  return (matches && matches[0]) ? matches[0] : null; 
  

  I am still not sure why you return null actually. PrepareHtml will return <a href=null></a>, so perhaps consider

  return (matches && matches[0]) ? matches[0] : ''; 
  

Anonymous functions kill kittens every day

• The 3 functions within your subscribe call are anonymous. It is in general terrible to look at a stack trace filled with anonymous functions
• You also have one in getEpisodeRange
• I do like that you declare your other functions 'the right way'

Comments go a long way

• getEpisodeRange needs comments, it is the most intense code
• There are actually no comments at all in this code, and I can parse everything just fine except the above mentioned function

Style

• Formatting looks good to me
• Naming looks good to me
• I did not find obvious repetitions from a DRY perspective

• Sreevisakh: Thanks very much for the reply. It is really helpful.. But Can you please review this from the reactive perspective as well. Because this is my first reactive program. I am not sure if this is the right way.
• Sreevisakh: I need a name for the a link as well. That is why I created the fileName function. I can only call the fileName function from my getEpisodeRange function but i couldn't find a way to pass that to the generateHtml function in the map chain.
• konijn: @Sreevisakh I think your usage is awesome, almost tutorial worthy. I will check the fileName thing and see if I can find a work-around.