Skip to content

Finished Assingment. Submitting for feedback.#14

Open
sri-go wants to merge 1 commit into
MUSA611-CPLN692-spring2020:masterfrom
sri-go:master
Open

Finished Assingment. Submitting for feedback.#14
sri-go wants to merge 1 commit into
MUSA611-CPLN692-spring2020:masterfrom
sri-go:master

Conversation

@sri-go

@sri-go sri-go commented Jan 27, 2020

Copy link
Copy Markdown

Hi Nathan,

I wanted feedback on the assignment. I did have some question regarding the efficiency of my code. The way I did the conversion of JSON to CSV seems inefficient to me. I essentially hardcoded the locations of the distinct fields into a pre-formatted array, but I realize that this isn't the best way. I was wondering what the other way to do this was. Part of my difficulty was understanding how do I sort the Key-Value pairs into their respective array.

Comment thread assignment/index.html
arr[8].push(a[i].WEBSITE);
arr[9].push(a[i].DENTAL_PHONE);
arr[10].push(a[i].FULL_ADDRESS);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You did this in a slightly unorthodox way (I'm not going to say it was wrong because I think there's a good argument to be made for representing a CSV just like this).

As for performance: I don't think you should sweat it and I don't think there is much room for improvement. Luckily for us, of the actual interpreter which underwrites modern javascript evaluation is highly optimized down to C (that's what Google's/node.js' V8 engine does).

As for readability, you might prefer something like this (and we might even want to dispense with the loops for readability's sake... more on that in a coming class):

// our header is here
var keys = Object.keys(a)
// create the array of arrays for the body
var body = []
// a bit of setup in anticipation of the loop ahead
for (key in keys) { body.push([]) }
for (var k = 0; k < keys.length in keys; k = k + 1) {
  for (var i = 0; i < a[i].length; i = i + 1) {
    var key = keys[k]
    body[k] = a[i][key]
  }
}
var result = [keys].concat(body)

@moradology moradology Jan 27, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming this code works (pretty sure, but I typed it in haste and didn't test it), it should generalize to key-value mappings regardless of the underlying keys or how many there are

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants