search

publiclab / simple-data-grapher

Turns CSVs into graphs in a few simple steps; embeds onto other websites!
https://publiclab.github.io/simple-data-grapher/
GNU General Public License v3.0
39 stars 52 forks source link

Refactoring Existing Code #57

Closed IshaGupta18 closed 5 years ago

IshaGupta18 commented 5 years ago

This PR aims to achieve 2 goals:

  1. Shift some parsing logic from View.js to CsvParser.js, making View.js completely independent of parsing logic.
  2. Till now, the CsvParser.js file had functions that were interdependent on each other ie. one function calling the other, which created problems in debugging and testing. Now, all the functions are independently written, each returning the required value and assigning to the variables. A functionHandler function calls all the functions in the required order. This way, if we get a bug, we can pinpoint the exact function which is causing a problem, instead of tracing the entire thing through function calls.

These changes will definitely increase the readability of the code for us as well as new-comers.

I think this is pretty much ready! @jywarren @namangupta01 @rexagod what do you think?

namangupta01 commented 5 years ago

Hi @IshaGupta18, can you look after the conflicts? Nice work btw.

IshaGupta18 commented 5 years ago

This PR can be closed I think, I extended this one for a bigger PR.

On Wed, Jun 26, 2019, 1:39 AM Naman Gupta notifications@github.com wrote:

Hi @IshaGupta18 https://github.com/IshaGupta18, can you look after the conflicts? Nice work btw.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/simple-data-grapher/pull/57?email_source=notifications&email_token=AJXHQZ5FDA5HELDBJKUH4WLP4J3PZA5CNFSM4H2WC52KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYROHUI#issuecomment-505603025, or mute the thread https://github.com/notifications/unsubscribe-auth/AJXHQZ6WTZIC7TMIAKQBB5TP4J3PZANCNFSM4H2WC52A .

IgorWilbert commented 5 years ago

Awesome work @IshaGupta18, thanks!