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

[IMP] Added Tests, Refactored Code and resolved some logical bugs #58

Closed IshaGupta18 closed 5 years ago

IshaGupta18 commented 5 years ago

This is in continuation of #57, but still, I'll copy the purpose of that PR:

  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.

Apart from the above, I have added some major tests for checking the parsing and data processing functions of CsvParser.js. I have also corrected some logical bugs in the code.

This is an important PR which aims to achieve many important goals. @jywarren @namangupta01 @gauravano @Souravirus @rexagod @IgorWilbert kindly have a look at it, and I hope we can merge it soon enough, as a lot of work will depend on this one! Thanks a ton!

IgorWilbert commented 5 years ago

Amazing work @IshaGupta18, thanks a lot! Since you merged #59, should we close this PR?

IshaGupta18 commented 5 years ago

Sure, go ahead! Thanks a lot!

On Sun, Jun 30, 2019, 6:44 PM igorwilbert notifications@github.com wrote:

Amazing work @IshaGupta18 https://github.com/IshaGupta18, thanks a lot! Since you merged #59 https://github.com/publiclab/simple-data-grapher/pull/59, should we close this PR?

— 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/58?email_source=notifications&email_token=AJXHQZ46HVQOEVU6PSFTG7LP5CWRTA5CNFSM4H2Y7QV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY4L6RI#issuecomment-507035461, or mute the thread https://github.com/notifications/unsubscribe-auth/AJXHQZZVGMICKKYHVGZQTDTP5CWRTANCNFSM4H2Y7QVQ .