GuyARoss/Review.md

## Review.md

      
    Raw
  

              Review.md
            
          
    Review

I have a little bit more time here today, so it hopefully will be a little more helpful.
Readme.md

How do I run your application? How can I contribute to your application? Pimp out this readme! Here is a repo of templates for you to use.
index.html

Woah! that is a lot of buttons, is there any way that we can automate the process of rendering these? outside of that, your semantics look good!
package.json

why is your entry point index.js? you don't even have an index.js file in your directory?
app.css

Source control handles versioning, most of the time if we have dead code, we can delete it before we check back in.
Also, I think for we should stick with either rgba or hsla, seems we are using both here. Additionally, some of you indentation is jacked, not sure what is going on there.
Calculator.js

We don't need the breaks after the returns, it is redundant because the code block is going to finalize anyways, which is what the eslint rule is yelling about. You can remove the eslint disable lines and the break keywords from :27-:43. I believe I spoke about this class yesterday, it still seems like there is a lot in here that isn't related to the class. I still think that we can pull the whole applyOperation method out, and just make it a util function instead, reworking your class around that refactor. But this is fine.
Running your application

Running with python3 -m http.server 8001
This thing seems broken..
so when i 2+8 it will equal 10 but when i then try to add 50 + 60 it will then equal 910.. Lets try to find the root cause and see if we can refactor some things on the way.
So from my understanding we are applying each of the values AFTER we click the operation? Why would we want to do that? Shouldn't we be applying immediately after we press the button? This leaves some messy mutation of state and weird logic that is happening.
Also, just from looking at what we are doing within our Button class, I think we should pull some of that logic out, it is doing a whole lot. I think that we should be creating 3 different buttons (apply value, calculate and apply operator) the inherit behavior from a base button, that way we can keep our classes nice and small and still follow the single responsibility principle (I linked this yesterday I believe), but basically if you didn't read it, the principle basically says that we should build out our classes in a way each class has one purpose.
Here is a small example demonstrating
class Button {
  // what are some things that a button does? 
  // - handles a press? 

  constructor(id, parentCalculator) {
    this.id = id
    this.parentCalculator = parentCalculator
  }

  handlePress() {
    throw Error('Not implemented')
  }
}

// our equal button
class EqualButton extends Button {
  constructor(id, parentCalculator) {
    super(id, parentCalculator)
  }

  handlePress() {
    this.parentCalculator.calculate()
  }
}
A "functional" approach

So far, we have been talking about OOP approaches to OOP code, this is a functional approach, where we have some global state that that we update with pure functions: (Really just something to think about, and possibly show off some new looking code).
const { state, dispatch } = store()

// I would call this on the equal button
const calculate = () => {
  const currentState = state()

  return applyOperator(
    state.operator,
    state.values
  )
}

// I would call this on each of the value buttons
const handleValue = (value) => dispatch({
  type: 'APPLY_VALUE',
  value
})

// I would call this on each of the operator buttons
const handleOperator = (value) => dispatch({
  type: 'APPLY_OPERATOR',
  value
})
where my store might looks something like this
const reducer = (state, { type, value }) => {
  switch (type) {
    case 'APPLY_VALUE': {
      return { ...state, values: [
        state.values[1],
        value,
      ]}
    }
    case 'APPLY_OPERATOR': {
      return { ...state, operator: value }
    }
    // could also put a reset in here to reset all of the values back to normal.
  }

  return state
}

const store = () => {
  let state = {
    values: [0,0]
    operator: ''
  }

  return {
    dispatch: (action) => {
      state = reducer(state, action)
    },
    state: () => state,
  }
}
So as you see int this example, the code is pretty straight-forward and easy to follow, no mutation of state outside of what is delegated by the physical store, etc.
Going to stop now, but good stuff, I can definitely see your code quality improve, looking forward to your next iteration.