2
\$\begingroup\$

This is my first ES6 & JSX React Component. It's just a simple counter with disabling/enabling buttons on certain conditions with notice.

I would like to know if this code is OK or if you could've done something another way. I want to avoid bad practices right from the beginning.

import React from 'react';

export default class Counter extends React.Component{
  constructor(props) {
    super(props);
    this.state = {
      number: 0,
    };

    this.decrement = this.decrement.bind(this);
    this.increment = this.increment.bind(this);
  }

  decrement() {
    this.setState({
      number: this.state.number - 1
    })
  }

  increment() {
    this.setState({
      number: this.state.number + 1
    })
  }

  disableIncrement() {
    if(this.state.number >= 10) {
      return true;
    } else {
      return false;
    }
  }

  disableDecrement() {
    if(this.state.number <= 0) {
      return true;
    } else {
      return false;
    }
  }

  notice() {
    if(this.disableIncrement()) {
      return 'You\'ve reached N° 10.';
    }
    if(this.disableDecrement()) {
      return 'You can\'t decrement. You\'re at N° 0';
    }
    return 'You can do both.';
  }

  render() {
    return(
      <div>
        <p>{this.state.number}</p>
        <button disabled={this.disableIncrement()} onClick={this.increment}>+</button>
        <button disabled={this.disableDecrement()} onClick={this.decrement}>-</button>
        <p>{this.notice()}</p>
      </div>
    )
  }
}
\$\endgroup\$

migrated from stackoverflow.com Oct 6 '17 at 18:49

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ it's ok, but usually things gets pretty complicated once you're dealing with multiple components; thus usually people handling the state between components using redux, relay, perhaps flux .. check it out \$\endgroup\$ – johnjerrico Oct 5 '17 at 9:15
  • \$\begingroup\$ I know, I want to use redux and other things, but I'd like to learn at least basic react before going into another stuff. \$\endgroup\$ – Café Oct 5 '17 at 10:46
1
\$\begingroup\$

Adding to Krisztian Balla's answer, you can use prevState to set your state as React state is updated in batches.

this.setState({
  number: this.state.number - 1
})

This should be changed to:

this.setState(prevState => ({
  number: prevState.number - 1
}));

For more info on prevState see the first answer here. lifecycle event state and prevState in react.js

\$\endgroup\$
  • \$\begingroup\$ Thanks, my code looks simpler, but I should use the prevState because the possibility of asynchronous updates, right? \$\endgroup\$ – Café Oct 5 '17 at 10:08
  • \$\begingroup\$ Yes. asynchronous updates can cause your program to not behave as expected. \$\endgroup\$ – Aanchal1103 Oct 5 '17 at 10:43
  • \$\begingroup\$ Yes. setState is async and this.state.someProperty inside it is not the reliable source of truth \$\endgroup\$ – dominik791 Oct 5 '17 at 10:51
2
\$\begingroup\$

First, you need to bind this to the notice function as well

this:

if(this.state.number <= 0) {
  return true;
} else {
  return false;
}

Can be written as:

this.state.number <= 0

as It will return those booleans anyway

in which case you could write:

notice() {
  const { number } = this.state

  if(number >= 10) {
    return 'You\'ve reached N° 10.';
  }

  if(number <= 0) {
    return 'You can\'t decrement. You\'re at N° 0';
  }

  return 'You can do both.';
}

Note the destructuring pattern:

const { number } = this.state
// is the same as
const number = this.state.number
\$\endgroup\$
  • \$\begingroup\$ Thank you very much. I'd like to ask something: First, you need to bind this to the notice function as well Please explain why. It works even without this.notice = this.notice.bind(this) in constructor. I thought I have to bind only functions that handle events. And second question. The disableIncrement/Decrement methods return the right boolean when i shorthanded them. But the buttons wont get disabled in the render function. Do you have any idea why? \$\endgroup\$ – Café Oct 5 '17 at 11:09
  • \$\begingroup\$ I ended up with this: gist.github.com/anonymous/fae8ad746e6c9c5ec43cac30749e407b What do you think? \$\endgroup\$ – Café Oct 5 '17 at 13:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.