Skip to content
This repository has been archived by the owner on Dec 28, 2017. It is now read-only.

Fix parser for examTimetable #14

Merged
merged 1 commit into from
Nov 6, 2016
Merged

Fix parser for examTimetable #14

merged 1 commit into from
Nov 6, 2016

Conversation

li-kai
Copy link
Member

@li-kai li-kai commented Nov 5, 2016

So NUS decided to use pdfs instead and this parses the pdfs. Also a change to requestCache by enabling caching of binary files.

Copy link
Member

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship it if it works! 😄


const fileArray = new Uint8Array(fileData);
return pdfjs.getDocument(fileArray)
.then((pdfDocument) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're gna use es6 for our parses then we better specify a node version in .nvmrc

// get text content items from all pages
return Promise.all(pages.map((page) => {
return page.getTextContent().then((content) => {
return content.items.slice(17, -1); // remove page numbers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can explain what 17 is? Try to reduce magic numbers.

// first 21 chars contain the date and time
const dateTime = moduleString.substr(-21).split(')');
// only keep numerics
const date = dateTime[0].replace(/\D/g,'');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: space after ,

const time = dateTime[1];

// remove date and time to parse module faculty, code and name
moduleArr.splice(-8, 8);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain what 8 means.

helpers.requestCached('https://webrb.nus.edu.sg/examtt/Exam' + options.academicYear.slice(0, 4) +
'/Semester ' + options.semester + '/MASTER Sem ' +
options.semester + ' by Module.html', options, function (err, data) {
var url = 'https://webrb.nus.edu.sg/examtt/Exam' + options.academicYear.slice(0, 4) +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using es6 template strings?

@yangshun
Copy link
Member

yangshun commented Nov 6, 2016

Can squash and merge if it's ready. Would be better if can squash, rebase then ff merge but Github doesn't provide that option out of the box :(

@li-kai li-kai merged commit 0af877c into master Nov 6, 2016
@li-kai li-kai deleted the 2016-fix branch November 6, 2016 04:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants