phetsims / plinko-probability

"Plinko Probability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 7 forks source link

Why create BinInterface and PegInterface #36

Closed jessegreenberg closed 8 years ago

jessegreenberg commented 8 years ago

Why were BinInterface and PegInterface abstracted into separate types? These functions aren't really general utility functions and it seems like they could be added directly to the prototypes of Histogram and GaltonBoard. If added to prototypes, would it simplify code at call sites? For instance,

    var axisCenterX = modelViewTransform.modelToViewX( BinInterface.getCenterX() );

it seems strange to get the centerX from a static function instead of from a particular model element.

veillette commented 8 years ago

@Denz1994:

The function in PegInterface should be added to the prototype of GaltonBoard. The pegInterface was used in Balls, BallsLayerNode and the GaltonBoardCanvasNode.

The GaltonBoardCanvasNode depends on the GaltonBoard, so you will have access to all the methods. The Ball Constructor can take the galtonBoard as parameter. The BallsLayerNode only need the spacing between two pegs, which is a trivial function in terms of the numberOfRows.

The BinInterface was used almost exclusively by HistogramNode, so we should attach it to the prototype of histogram. It is also used by the CylindersBackNode and CylinderFrontNode, but we need only of function from the BinInterface. We could bind that function and pass it in an argument of the constructor.

veillette commented 8 years ago

Good point @jessegreenberg . It is wholly unnecessary to have these interfaces.

Assigning to @Denz1994

Denz1994 commented 8 years ago

I was able to get rid of the bin/peg interfaces and moved all the methods into their respective call locations. For example, instead of referencing the peg interface for a method, the method is already included within the file. This removes dependencies on pegInterface.js and binInterface.js and thereby have been deleted from the directory.

Assigning to @jessegreenberg for review and input.

jessegreenberg commented 8 years ago

Above commits look great, thanks @Denz1994. Closing.