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))
.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); } ## 1 Answer 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'

• 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
• 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 Jul 12 '16 at 7:46
• 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. – Sreevisakh Jul 12 '16 at 7:58
• @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. – konijn Jul 12 '16 at 23:14