LoginSignup
32
34

More than 5 years have passed since last update.

@wintyo

Reactで読みやすいコードを書く

Last updated at Posted at 2018-09-17

始めに

皆さんは普段Reactはどのように書いているでしょうか？それぞれ書き方があると思いますし、多少の違いはその人の個性ってことでいいかなと思っていました。ただ最近になって思っていたより他の人と書き方が違うことに気づき、本当に自分の書き方はいいのだろうかと疑問になってきました。そこでなんで自分がこのような書き方をしているか改めて考えなおすことにしました。
全部書くのは大変だったので特に特徴的なところだけ載せました。参考にしてもらえると幸いです。

僕が目指す読みやすいコード

class内のコードは重要なもののみ残す

ReactのコンポーネントはReact.Componentから継承して使っていますが、classは閉じたグローバル変数とも言われるように、class内に書かれたものはどこでもstateやメンバ変数を書き換えることができます。したがってclass内のコードが増えるほどどこで書き換え処理が行われているか追っていくのが辛くなります。stateやメンバ変数が変化しない処理は外に出してあげたほうが重要なコードだけ残って読みやすくなると思います。
具体的には以下のようなルールで書いています。

Elementを返すメソッドは作らずコンポーネントにする

Elementを返すメソッドを作ってrender部分を読みやすくするという方法があると思いますが、それをするくらいならStateless Componentを作ったほうがいいです。
メソッドにした場合はなんのデータを使って作られたものか分かりにくくなり、結局そのコードの実装を見ないと挙動が分からないのであまり意味がないです。

BAD
import React, { Component } from 'react';

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      buttonText: 'click me!'
    };
    this.onClick = this.onClick.bind(this);
  }

  onClick() {
    console.log('click!');
  }

  createButtonElement() {
    const { buttonText } = this.state;
    return (
      <button onClick={this.onClick}>{buttonText}</button>
    );
  }

  render() {
    return (
      <div>
        {/* 上手く分離させたように見えるが、 */}
        {/* 実際はthis.state.buttonTextによって表示内容が変わっている。 */}
        {/* またコールバックとしてonClickも使われているが、それはこのメソッドの中身を見ないと分からない */}
        {this.createButtonElement()}
      </div>
    );
  }
GOOD
import React, { Component } from 'react';
import PropTypes from 'prop-types';

const Button = (props) => {
  const { buttonText } = props;
  return (
    <button onClick={props.onClick}>{buttonText}</button>
  );
};
Button.propTypes = {
  buttonText: PropTypes.string,
  onClick: PropTypes.func
};

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      buttonText: 'click me!'
    };
    this.onClick = this.onClick.bind(this);
  }

  onClick() {
    console.log('click!');
  }

  render() {
    const { buttonText } = this.state;
    return (
      <div>
        {/* ButtonというコンポーネントにbuttonTextとonClickを渡していて */}
        {/* これらが関係していることが目に見えて分かる */}
        <Button
          buttonText={buttonText}
          onClick={this.onClick}
        />
      </div>
    );
  }
}

stateなどの状態を変えないただの計算メソッドは外に出す

stateにあるリストの合計を求めるとか、stateを更新せずただ計算するだけのメソッドはclassの外に書いたほうがいいです。引数をかかないとなんのデータから算出されたものか分からないですし、そもそもclassに入っている必要がないです。

BAD
class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      list: [1, 2, 3, 4, 5]
    };
  }

  /**
   * 配列の合計を求める
   * @return {number} - 合計値
   */
  calcTotal() {
    const { list } = this.state;
    let total = 0;
    for (let i = 0; i < list.length; i++) {
      total += list[i];
    }
    return total;
  }

  render() {
    // なんのデータから算出された合計かは実装見ないと分からないし、汎用性がない
    const total = this.calcTotal();
    return (
      <div>
        <span>{total}</span>
      </div>
    );
  }
}
GOOD
/**
 * 配列の合計を求める
 * @param {Array<number>} arr - 数値の配列
 * @return {number} - 合計値
 */
function calcTotal(arr) {
  let total = 0;
  for (let i = 0; i < arr.length; i++) {
    total += arr[i];
  }
  return total;
}

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      list: [1, 2, 3, 4, 5]
    };
  }

  render() {
    const { list } = this.state;
    const total = calcTotal(list);  // listを使って合計を求めているのが一目で分かる
    return (
      <div>
        <span>{total}</span>
      </div>
    );
  }
}

内部変数を必要とする処理は出来るだけクラスに押し込む

timerIdとか、表示には使わないが処理に必要な内部変数をコンポーネントに持っていると管理が大変になってきます。そもそも内部変数を必要とする処理は単純でもそれなりにコードを書いてしまいます。出来るだけクラスを作ってそっちに管理させたほうがコンポーネント内はスッキリすると思います。

BAD
class App extends React {
  constructor(props) {
    super(props);

    // pollingをするためにtimerIdとstartPolling, stopPollingを用意してコードが少し増えた
    this.timerId = null;
    this.startpolling();
  }

  startPolling() {
    // ポーリングする関数
    const pollingAction = () => {
      console.log('polling');
      this.timerId = window.setTimeout(pollingAction, 1000);
    };
    pollingAction();
  }

  stopPolling() {
    window.clearTimeout(this.timerId);
  }

  componentWillUnmount() {
    this.stopPolling();
  }
}
GOOD
const noop = () => {};

/**
 * Pollingするためのクラスを作る
 */
class Polling {
  constructor(pollingAction = noop, interval = 1000) {
    this.pollingAction = pollingAction;
    this.interval = interval;
    this.timerId = null;
  }

  start() {
    // ポーリングする関数
    const action = () => {
      this.pollingAction();
      this.timerId = window.setTimeout(action, interval);
    };
    action();
  }

  stop() {
    window.clearTimeout(this.timerId);
  }
}

class App extends React {
  constructor(props) {
    super(props);

    // ポーリングの設定がこれだけで済む
    this.polling = new Polling(() => {
      console.log('polling');
    }, 1000);
    this.polling.start();
  }

  componentWillUnmount() {
    this.polling.stop();
  }
}

機能のメソッドをグループ化する

コンポーネントにあるメソッドが増えると動作を追うのが大変になってきます。そこでメソッドをthisの直下に置かず、オブジェクトにしてグループ化します。プロパティに直接セットする書き方はbabelの場合はbabel-preset-stage-2を入れておく必要があります。オブジェクトで定義するとついでにアロー関数で定義できるのでbindを書く必要もなくなります。
グループ化すると全体が分かりやすくなり、無関係な操作グループはエディタで閉じることも出来るので読みやすくなると思います。

BAD
// 全部フラットにメソッドを定義していると数が多くて辛くなってくる
class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      isMenuOpen: false,
      text: '',
      numberText: '',
    };

    // bindするのも嫌になってくる
    this.openMenu = this.openMenu.bind(this);
    this.closeMenu = this.closeMenu.bind(this);
    this.fetchList = this.fetchList.bind(this);
    this.requestChangeList = this.requestChangeList.bind(this);
    this.onSubmit = this.onSubmit.bind(this);
    this.onBackButtonClick = this.onBackButtonClick.bind(this);
    this.onNextButtonClick = this.onNextButtonClick.bind(this);

    this.fetchList();
  }

  openMenu() {
    this.setState({ isMenuOpen: true });
  }

  closeMenu() {
    this.setState({ isMenuOpen: false });
  }

  fetchList() {
    this.props.dispatch(fetchList());
  }

  requestChangeList(text, index) {
    this.props.dispatch(requestChangeList(text, index));
  }

  onSubmit() {
    const { text, numberText } = this.state;
    this.requestChangeList(text, +numberText);
  }

  onBackButtonClick() {
    this.props.history.back();
  }

  onNextButtonClick() {
    this.prop.history.push('/next');
  }

  onChangeText(e) {
    this.setState({
      text: e.target.value
    });
  }

  onChangeNumber(e) {
    this.setState({
      number: e.target.value
    });
  }

  render() {
    const { text, numberText, isMenuOpen } = this.state;
    const { list } = this.props;
    return (
      <div>
        <ul>
          {list.map((item, index) => (
            <li key={index}>{item}</li>
          ))}
        </ul>
        <form onSubmit={this.onRequestChangeList}>
          <input type="number" value={numberText} onChange{this.onChangeNumber} />
          <input type="text" value={text} onChange={this.onChangeText} />
        </form>
        <Menu isOpen={isMenuOpen}>
          <button onClick={this.closeMenu}>メニューを閉じる</button>
          <button onClick={this.onBackButtonClick}>back</button>
          <button onClick={this.onNextButtonClick}>next</button>
        </Menu>
      </div>
    );
  }
}
GOOD
// クラスのプロパティに直接セットするにはbabel-preset-stage-2がいる
class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      isMenuOpen: false,
      text: '',
      numberText: '',
    };

    this.api.fetchList();
  }

  /**
   * メニュー関係の操作
   */
  menu = {
    open: () => {
      this.setState({ isMenuOpen: true });
    },
    close: () => {
      this.setState({ isMenuOpen: false });
    }
  };

  /**
   * API関係の操作
   */
  api = {
    fetchList: () => {
      this.props.dispatch(fetchList());
    },
    requestChangeList: (text, index) => {
      this.props.dispatch(requestChangeList(text, index));
    }
  };

  /**
   * ページ関係の操作
   */
  page = {
    back: () => {
      this.props.history.back();
    },
    next: () => {
      this.props.history.push('/next');
    }
  };

  /**
   * コールバック関数
   */
  callbacks = {
    onSubmit: () => {
      const { text, numberText } = this.state;
      this.api.requestChangeList(text, +numberText);
    },
    onChangeText: (e) => {
      this.setState({
        text: e.target.value
      });
    },
    onChangeNumber: (e) => {
      this.setState({
        number: e.target.value
      });
    }
  };

  render() {
    const { text, numberText, isMenuOpen } = this.state;
    const { list } = this.props;
    return (
      <div>
        <ul>
          {list.map((item, index) => (
            <li key={index}>{item}</li>
          ))}
        </ul>
        <form onSubmit={this.callbacks.onRequestChangeList}>
          <input type="number" value={numberText} onChange{this.callbacks.onChangeNumber} />
          <input type="text" value={text} onChange={this.callbacks.onChangeText} />
        </form>
        <Menu isOpen={isMenuOpen}>
          <button onClick={this.menu.close}>メニューを閉じる</button>
          <button onClick={this.page.back}>back</button>
          <button onClick={this.page.next}>next</button>
        </Menu>
      </div>
    );
  }
}

できるだけ1行だけで分かるようにする

前後の行を見たら分かるとかではなく、その行を見ただけでやっていることが理解できるようなコードにしたいです。要はその行だけだと複数の可能性が考えられそうなコードはできるだけ書かないように目指します。

Elementを変数に入れない

変数はなんでも入れられるので、ただでさえ数値、オブジェクト、配列、関数、インスタンス、クラスと色々な可能性を考えなければいけないのに、さらにElementまでくるとなかなか大変です。どうしても必要な場合は後ろにElementとかNodeとかつければまだわかりますが、ついていなかったらもうなんのデータか分からないです。

BAD
render() {
  const list = [];
  for (let i = 0; i < 5; i++) {
    list.push(<li key={i}>item{i}</li>);
  }
  return (
    <ul>
      {/* 配列を展開したらエラーになるのではと誤解しやすい（実際はElementの配列なので表示できる） */}
      {list}
    </ul>
  );
}
GOOD
import { times } from 'lodash';

render() {
  return (
    <ul>
      {/* 変数でキャッシュしても仕方ないので直接書く */}
      {times(5).map((index) => (
        <li key={index}>item{index}</li>
      ))}
    </ul>
  );
}

propsで関数だけは変数展開せずそのまま使う

上で書いたように変数に入るパターンを減らすためにconst { hoge } = this.props;のようなコードでは関数を受け取らないようにしています。関数を実行する場合はそのままthis.props.onHoge();のようにしています。こう書くと上で書いた計算メソッドとの区別も出来ます。

BAD
function calcTotal(arr) { ... }

const Component = (props) => {
  // 展開された変数が関数なのかただの値なのかの区別がつきにくい
  const { list, onClick, buttonText } = props;
  // 展開する変数が多くなるとpropsからの関数なのかグローバルの関数なのか区別がつきにくくなる
  const total = calcTotal(list);
  return (
    <div>
      <div>{total}</div>
      <button onClick={onClick}>{buttonText}</button>
    </div>
  );
};
GOOD
function calcTotal(arr) { ... }

const Component = (props) => {
  // ここで書くのは値だけ
  const { list, buttonText } = props;
  // propsにある関数を展開していないのでグローバル関数であることがわかる
  const total = calcTotal(list);
  return (
    <div>
      {/* props.があるのでpropsで受け取る関数であることが明白 */}
      <button onClick={props.onClick}>{buttonText}</button>
    </div>
  );
};

reduxでdispatchは省略せずに書く

react-reduxでconnectとかでdispatchを省略して書けるようにできますが、それをせずdispatchは明記させます。これでその行をみただけでstoreにdispatchしていることが明白になります。

BAD
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';

function increment() {
  return {
    type: INCREMENT
  };
}

class App extends Component {
  constructor(props) {
    super(props);
    this.onClick = this.onClick.bind(this);
  }

  onClick() {
    // storeにdispatchしているのかが分かりにくい（親コンポーネントに通知しているだけとも受け取れる）
    this.props.increment();
  }

  render() {
    const { count } = this.state;
    return (
      <div>
        <div>{count}</div>
        <button onClick={this.onClick}>increment</button>
      </div>
    );
  }
}
App.propTypes = {
  count: PropTypes.number,
  increment: PropTypes.func
};

function mapStateToProps(state) {
  return {
    count: state.count
  };
}

export default connect(mapStateToProps, {
  increment
})(App);
GOOD
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';

function increment() {
  return {
    type: INCREMENT
  };
}

class App extends Component {
  constructor(props) {
    super(props);
    this.onClick = this.onClick.bind(this);
  }

  onClick() {
    // dispatchしているのが明白
    this.props.dispatch(increment());
  }

  render() {
    const { count } = this.props;
    return (
      <div>
        <div>{count}</div>
        <button onClick={this.onClick}>increment</button>
      </div>
    );
  }
}
App.propTypes = {
  count: PropTypes.number,
  dispatch: PropTypes.func
};

function mapStateToProps(state) {
  return {
    count: state.count
  };
}

export default connect(mapStateToProps)(App);

最後に

色々書き方を説明しましたが、結局僕は以下の点を気にしているようです。

  • クラスComponent内のコードは重要なもののみ残し、できるだけコード量を減らす
    • 減らすのが難しそうなら構造化する
  • その行だけでどういう操作をしているのか分かるコードにする

もし他にいい書き方とかあればコメントしてもらえると嬉しいです。

32
34
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
32
34