Dice-rolling tool

Question

I am an experienced programmer, but a complete beginner with React. My goal is to teach myself React.

I've made a simple die-roller application. The application presents a drop-down, and the user can choose what die to roll (e.g. a d6 is a six-sided die). When the user clicks "roll", an appropriate random number is generated and displayed.

The app is working fine, but I would be grateful to know whether I've written genuinely "React-y" code; whether it's structured properly, whether I have got the naming conventions right, etc.

Some specific questions: is the "Dice" constant declared in the right place/way? Should the "option" elements be full React components? Should the "button" be a full React component?

import React, { Component } from 'react';
import './App.css';

const Dice = ["4", "6", "8", "10", "12", "20"];

class App extends Component {

  constructor(props) {
    super(props);
    this.state = {
      outcome: null,
      sides: null
    };
    this.handleClick = this.handleClick.bind(this);
    this.setSides = this.setSides.bind(this);
  }

  render() {
    return (
      <div className="App">
        <header className="App-header">
          Die roller
        </header>

        <DieChooser dice={Dice} setSides={this.setSides}></DieChooser>
        <button onClick={this.handleClick}>Roll</button>
        <Result outcome={this.state.outcome} />

      </div>
    );
  }

  setSides(s) {
    this.setState({sides: s});
  }

  handleClick() {
    const rolled = Math.floor(Math.random() * (this.state.sides)) + 1;
    this.setState({ outcome: rolled })
  }
}

class DieChooser extends Component {

  constructor(props) {
    super(props);
    this.handleChange = this.handleChange.bind(this);
    this.props.setSides(props.dice[0]);
  }

  handleChange(e) {
    this.props.setSides(e.target.value)
  }

  render() {
    return (
      <select onChange={this.handleChange}>
        {this.props.dice.map((die) =>
          <option key={die} value={die}>d{die}</option>
        )}
      </select>
    )
  }
}

function Result (props) {
  return (
    <p>{props.outcome}</p>
  )
}

export default App;

Answer 1

With the new ES6 syntax, arrow functions allow you to remove .bind from your code.

Don't make a stateful component if you're not using its state. Your DieChooser doesn't handle its value change by itself since its parent component is handling it. Make a stateless(function) component whenever you can and do not require to use lifecycle methods or a private state.

I also moved the setSides up to the parent to improve readability.

A select tag will always select the first option by default; there is no need for this.props.setSides(props.dice[0]);.

Props deconstructing will also make your code more readable, as the last component Result only takes a single line now.

Working live example :

const Dice = [4, 6, 8, 10, 12, 20];

class App extends React.Component {

	constructor(props) {
		super(props);

		this.state = {
			outcome: null,
			sides: null
		};
	}

	render = () => {
		return (
			<div className="App">
				<header className="App-header">
					Die roller
                </header>
				<DieChooser dice={Dice} setSides={this.setSides} />
				<button onClick={this.handleClick}>Roll</button>
				<Result outcome={this.state.outcome} />
			</div>
		);
	}

	setSides = event => {
		this.setState({ sides: event.target.value });
	}

	handleClick = ev => {
		const rolled = Math.floor(Math.random() * (this.state.sides)) + 1;
		this.setState({ outcome: rolled })
	}
}

const DieChooser = ({ dice, setSides }) => {
	return (
		<select onChange={setSides}>
			{dice.map( die =>
				<option key={die} value={die}>d{die}</option>
			)}
		</select>
	)
}

const Result = ({ outcome }) => <p>{outcome}</p>


ReactDOM.render(<App/>, document.getElementById('root'))

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.3.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.3.0/umd/react-dom.production.min.js"></script>
<div id='root'/>