Full Initial Code Review

[mrk3767]

No description provided.

[RobertWarrenGilmore]

I haven't dug into the various components yet, but here is some feedback on what I've seen so far.

[RobertWarrenGilmore]

As discussed offline, I dislike the way this method is written.

I see a superficial similarity to the Flux design pattern - all "actions" go through a single handler and are redirected from there. I think the way it redirects squanders any benefit you get from that pattern, and I'm not just talking about the eval(). Even if you mapped the action's stated function name to a function without eval(), this universal handler would still be kind of a sham. The various buttons would really have various handlers that they name explicitly. The advantage of having a main handler is that your buttons don't need to be aware of which method is going to handle the click action. If you're going to do Flux, go all the way. This handler should take in an action that expresses an intention to trigger a change the application's state, with an enum and parameters, and then the handler (with helpers, perhaps) should opaquely decide what the new state is based on that intention. Note that the action expresses an intention to trigger a change - it doesn't express the whole new state or the specific method that should be called to change it. After this handler finishes updating the state, it should call a master rendering method that takes the whole application's state object and decides what to render.

Alternatively, you could formalise what you're doing with this eval() by giving each button a different handler. You're basically doing that already, but binding the various functions directly to their buttons seems cleaner than eval(). That may involve putting various handler functions in this file that each call a function in a different file. It's not fluxy, but it makes the handlers more explicit.

[RobertWarrenGilmore]

Another alternative is to formalise this pattern as your take on Flux and call it Evalux. 😆

[mrk3767]

Relates to #2

[RobertWarrenGilmore]

Consider configuring your editor to clean excess whitespace.