optimizely / nuclear-js

Reactive Flux built with ImmutableJS data structures. Framework agnostic.
https://optimizely.github.io/nuclear-js/
MIT License
2.23k stars 141 forks source link

Use clearer variables names in the example #30

Closed appsforartists closed 9 years ago

appsforartists commented 9 years ago

In the example, the taxPercent store is setup thusly:

this.on('setTaxPercent', function(percent, value) {
  return value
})

percent is a confusing name, since presumably both values are percentages. Since this is an example, clarity is especially important. I suggest state and payload, to maintain the convention used throughout the rest of the document. oldPercent and newPercent would also be good choices.

appsforartists commented 9 years ago

Similarly, getTotal should be called totalGetter, to match the preceding section:

var over100Getter = [
  getTotal,
  function(total) {
    return total > 100
  }
]

reactor.observe(over100Getter, function(isOver100) {
  if (isOver100) {
    alert('Shopping cart over 100!')
  }
})
appsforartists commented 9 years ago

And this should be reactor.ReactMixin:

var React = require('react')

var ShoppingCart = React.createClass({
  mixins: [react.ReactMixin],
appsforartists commented 9 years ago

syncronously should be synchronously.

appsforartists commented 9 years ago

Utilties should be Utilities

jordangarcia commented 9 years ago

Hi!

Thanks for suggesting these changes. Feel free to open a PR and I will gladly merge.

Otherwise I will make these changes later this week.