Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions googleSpreadsheet.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
const google = require('googleapis');
const keys = require('./config/keys');

function rowToObject(val, lab, index) {
const o = {};
for (let i = 0; i < lab.length; i += 1) {
o[lab[i]] = val[i];
// creates a dictionary mapping column names with the values
// ex: { floor : 402, business : coworking, etc..}
function spreadsheetValuesToObject(values, columns) {
const formatedRow = {};
for (let i = 0; i < columns.length; i += 1) {
formatedRow[columns[i]] = values[i];
}
o.cellRef = 'https://docs.google.com/spreadsheets/d/1QHKa3vUpht7zRl_LEzl3BlUbolz3ZiL8yKHzdBL42dY/edit#gid=0&range=A' + index + ':T' + index;
return o;
return formatedRow;
}
Copy link
Member

Choose a reason for hiding this comment

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

not a good practice to mix refactoring and altering behaviour ( i.e. deleting cellRef)

Copy link
Member

Choose a reason for hiding this comment

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

and now the template for http:///recent display irrelevant links to home page.


function loadDatabase(callback) {
Expand All @@ -21,8 +22,10 @@ function loadDatabase(callback) {
console.log(`The API returned an error: ${err}`);
return;
}
return callback(response.values.map((row, index) =>
rowToObject(row, response.values[0], index + 1)).splice(1));
const columns = response.values[0];
// map function transforms a list into another list using the given lambda function
const formatedRows = response.values.map(row => spreadsheetValuesToObject(row, columns));
return callback(formatedRows);
});
Copy link
Member

Choose a reason for hiding this comment

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

introducing redundant local variable is not recommended. It can be a symptom of poor function breakdown

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually meant to break into two steps for better readability,
All the things where nested into one line of code that was a headache to read

Copy link
Member

Choose a reason for hiding this comment

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

Extracting local variable is usually bad design.
Extracting subfunction is fine (while inlining others maybe)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what "Extracting subfunction" means.
If javascript is smart (and I assume it is), formatedRows is passed by reference.
It does improve readbility don't you think so ?

Copy link
Member

Choose a reason for hiding this comment

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

}

Expand Down