HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptrailsMinor

React.js components for a golf app

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
golfappreactcomponentsfor

Problem

I've recently started to look into React. I feel like I'm understanding it, but I want to make sure I'm doing the best practices before I make too many bad habits.

I'm using the react-rails gem, and my data structure is as follows:

GolfCourse:

has_many :holes


Hole:

belongs_to :golf_course
has_many :yardages


Yardage:

belongs_to :tee_box


I load the component using:



The idea behind the component is: The user has loaded the page to enter their recent golfing score, the page loads the scorecard for the current course they have selected. The user then changes the dropdown to select which tee box they played off.

The scorecard yardage/par/stroke_index will change depending on which tee colour they have chosen.

Scorecard Component

var Scorecard = React.createClass({

  getInitialState: function() {
    return {
      yardages: {}
    };
  },

  changeYardage: function(teeBoxId) {
    this.setState({
      yardages: _.filter(this.props.yardages, function(yardage){ return yardage.tee_box_id == teeBoxId; })
    });
  },

  renderHoles: function() {
    var state = this.state;
    return this.props.holes.map(function(option) {
      yardage = _.find(state.yardages, function(yardage) { return yardage.hole_id == option.id; });

      return (
        
          {option.number}
          {option.name}
          {typeof(yardage) !== "undefined" ? yardage.yards : ""}
          {typeof(yardage) !== "undefined" ? yardage.par : ""}
          {typeof(yardage) !== "undefined" ? yardage.stroke_index: ""}
          
        
      );
    });
  },

  render: function() {
    return (
      
        

        
          
            
              Hole
              Yards
              Par
              Stroke Index
              Score
            
          
          
            {this.renderHoles()}
          
        
      
    );
  }
});


Tee box select component

```
var TeeBoxSelect = React.createClass({
getInitialState: funct

Solution

Your component is pretty good. I even had to look at this guide to check when props and state should be used, and your code pretty much nailed it. There are a few things to change though.

First is to not rely on the fact that props and state get handed down to your component. It's best if you provide default props the same way you did with state. That way, you're always sure they have values. It also serves as pseudo-documentation, telling the developer that's lost and found his way into your component that this component expects the following props and state.

So... changeYardage selects a yardage, a single yardage. It makes no sense keeping it an array and end up using find later on. You can use find on changeYardage instead so that yardages stores an object instead of an array. If find returns undefined, just default it to a blank object. And since it's a single yardage object, we rename it to a singular yardage instead.

this.setState({
  yardage: this.props.yardages.find(yardage => yardage.hole_id === option.id) || {};
});


I notice you use React, so probably you're using Babel (if not, you should). This gives you access to a lot of shortcuts seen above. First, in ES6, arrays now have a find method so you can drop using lodash/underscore for that. Then there's the arrow functions to make callbacks a bit shorter. Not related to ES6 syntax, you should also use strict comparison, unless the comparison calls for loose comparison.

Now with regards to the yardages table, I'm not sure you want to actually show a blank table when yardage is not defined. You might want to actually just show an error message instead of a blank table.

And I notice you use BEM convention for CSS. Good. At least you are not using inline styles, because it has a very high specificity and makes it hard for stylesheets to override.

Code Snippets

this.setState({
  yardage: this.props.yardages.find(yardage => yardage.hole_id === option.id) || {};
});

Context

StackExchange Code Review Q#108985, answer score: 2

Revisions (0)

No revisions yet.