phetsims / trig-tour

"Trig Tour" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/trig-tour
GNU General Public License v3.0
2 stars 2 forks source link

Change special angle arrays to object literal #60

Closed jessegreenberg closed 9 years ago

jessegreenberg commented 9 years ago

Issue related to #34. The special angle arrays are created in ReadoutNode and encode the information needed by FractionNode to build the nodes representing the fractional form of special angles evaluated by each trig function. For instance:

    this.sinFractions = [
      [ 0, '' ],
      [ 1, 2 ],
      [ 'q' + 2, 2 ],
      [ 'q' + 3, 2 ],
      [ 1, '' ],
      [ 'q' + 3, 2 ],
      [ 'q' + 2, 2 ],
      [ 1, 2 ],
      [ 0, '' ],
      [ -1, 2 ],
      [ '-q' + 2, 2 ],
      [ '-q' + 3, 2 ],
      [ -1, '' ],
      [ '-q' + 3, 2 ],
      [ '-q' + 2, 2 ],
      [ -1, 2 ],
      [ 0, '' ]
    ];

is the structure for special angles of sin.

Rather than using string flags like '-' and 'q', I think it would be much more maintainable to restructure this object in a literal. I would like to see something like:

// object containing information needed by FractionNode to build the fractional form of cos evaluated at special
// angles - the first keys are the angle in degrees which are represented by the fraction.
SPECIAL_COS_FRACTIONS: {
      0: {
        numerator: 1,
        denominator: '',
        radical: false
      },
      30: {
        numerator: 3,
        denominator: 2,
        radical: true
      },

and so on to represent the numerator, denominator, and radical flag. This would

1) Make it implicitly clear that object in the nested literal corresponds to an angle evaluated by the trig function. 2) Reduce the code complexity in FractionNode for parsing strings and looking for flags 3) Object key lookup reduces the loops necessary in ReadoutNode

jessegreenberg commented 9 years ago

I did this in the commit above. This was a pretty aggressive refactor so I triple checked behavior against dev.17. All is well and I think this is a more maintainable solution. Closing.