calband / calchart-viewer

An online show viewer and continuity generator for Calchart.
calband.github.io/calchart-viewer/
2 stars 2 forks source link

Clean up code? #131

Closed brandonchinn178 closed 9 years ago

brandonchinn178 commented 10 years ago

Possibly clean up PDF Generator code. Specifically, try to make box objects less complicated and extract the draw function into the actual function. Box objects should only know about themselves, we don't need to make them worry about drawing movements or continuities or anything. KEEP MODULAR

noahsark769 commented 10 years ago

You read my mind

brandonchinn178 commented 9 years ago

We can also release the PDF Generator without cleaning up code. We'll clean up while people test the PDFs

brandonchinn178 commented 9 years ago

@noahsark769 Have any comments on how to clean up code? Obviously, as the writer, I think the style is fine for the code, but the code is not clean... Started it a bit on the branch pdf__clean_up

noahsark769 commented 9 years ago

Hm, possibly I might think of extracting the drawing code for each widget (ic, imd, bev, sd) into a real class in its own file, that way you could extract some functionality into a base class (for example, have a PDFWidget class and a IndividualContinuityWidget and BirdsEyeViewWidget that extends it, and both would have a draw() method). That would make it cleaner for adding options as well.

Other than that, I bet you could write some utility functions for PDF drawing. I can do a thorough review of the code later and offer suggestions as well On Mon, Nov 24, 2014 at 5:20 PM Brandon Chinn notifications@github.com wrote:

@noahsark769 https://github.com/noahsark769 Have any comments on how to clean up code? Obviously, as the writer, I think the style is fine for the code, but the code is not clean... Started it a bit on the branch pdf__clean_up

— Reply to this email directly or view it on GitHub https://github.com/calband/calchart-viewer/issues/131#issuecomment-64295344 .

brandonchinn178 commented 9 years ago

@noahsark769 Can you see what I've done with the IndividualContinuityWidget class? Something like that?

noahsark769 commented 9 years ago

Yeah I like that! I would definitely add more description in the comments about what a widget is in general and how it should function, but the pattern looks very good