HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Input a web page with poker events, output a gcal compatible CSV file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pokerfileeventswithcsvoutputcompatibleinputpageweb

Problem

This script takes input from poker tournament pages at pokeratlas dot com, and outputs a CSV file that I can import to Google Calendar, listing each of the tournament events, start time, location, and a link back to pokeratlas for their full detail page.

I've used moment, Q, and cheerio. I learned moment specifically for parsing date/time strings here. I would use "jsdom" for parsing, because I've used it before, but it's a serious bear to install in Windows. So, this is the first time I've used anything jQuery like. I'm learning Q to use in other projects, so I thought I'd include it here, but I can't really find much of a good use for it, since there's very little that is actually asynchronous here.

I'm seeking any input anyone might have as to how to make better use of cheerio, Q, or general node.js code criticism.

```
var http = require('http');
var moment = require('moment');
var Q = require('q');
var cheerio = require('cheerio');

function loadPage(options) {
var content = "",
d = Q.defer();
var req = http.request(options, function(res) {
res.setEncoding("utf8");
res.on("data", function(chunk) {
content += chunk;
});
res.on("end", function() {
d.resolve(content);
});
res.on("error", function(err) {
d.reject(err);
});
});
req.end();
return d.promise;
}

function parseSection(section) {
// each day has a "section" tag, which has a child that has class "header" and it's "id" == "header-(date)" where date is the date of the events contained.
var date = section.children(".header").attr("id").substr("header-".length);
// each section has multiple divs classed "tournament-item", which give the details of each individual tournament
var tournaments = section.find(".tournament-item");
tournaments.each(function(index, tournament) {
parseTournament(date, cheerio(tournament));
});
}

function parseTournament(date, to

Solution

Bearing in mind that this code was written over five years ago, things may have changed drastically with it and you may have learned a lot since then.
Long comment lines

The longest line appears to be this comment which is 159 characters (including whitespace):

// each day has a "section" tag, which has a child that has class "header" and it's "id" == "header-(date)" where date is the date of the events contained.


For readability, use a multi-line comment and split it into multiple lines:

/* each day has a "section" tag, which has a child that has class "header" and 
     it's "id" == "header-(date)" where date is the date of the events contained.
  */


http.request() vs http.get()

Using http.get() eliminates the need to call req.end();

Since most requests are GET requests without bodies, Node.js provides this convenience method. The only difference between this method and http.request() is that it sets the method to GET and calls req.end() automatically. 1

Promise library

The Q library hasn't been published since 2018 and has failed builds. It isn’t needed to return a promise- this SO post shows an example using the Promise constructor. That could easily be used to eliminate the Dependency of the Q library.
overwriting variables

I see these lines:

if(time) {
   time = moment(date + " " + time, "YYYY-MM-DD H:mma");
   time = time.format("hh:mm:ss A");
}


While the intermediate value (returned by calling moment() is only used once, it isn’t exactly the same as the First value assigned to time because it is a moment object instead of a string. For readability it would be better to call that variable something else, like dateTime.

Code Snippets

// each day has a "section" tag, which has a child that has class "header" and it's "id" == "header-(date)" where date is the date of the events contained.
/* each day has a "section" tag, which has a child that has class "header" and 
     it's "id" == "header-(date)" where date is the date of the events contained.
  */
if(time) {
   time = moment(date + " " + time, "YYYY-MM-DD H:mma");
   time = time.format("hh:mm:ss A");
}

Context

StackExchange Code Review Q#82168, answer score: 2

Revisions (0)

No revisions yet.