t-morisawa/scamper

Reducerのリファクタリング

Closed this issue · 2 comments

t-morisawa commented

ツッコミどころ

  • idじゃなくてindexだと思う
  • indexを状態に持たせるのか、元となってる配列をstateに持たせるのか
    • 現状、presentational containerであるinteractionがconstからデータを引っ張ってくるような作りになっており、違和感がある
    • こういうのはせめてコンテナコンポーネントが処理するだけで、presentationalは受け取ったものを表示するだけにしたい
    • まあstateに全部持たせちゃうのが良いと思う
  • 関数名がセンスない
    • なんで状態を表す名前になっているのか
    • isStart -> starter
    • resultIs -> resultSwitcher
    • data -> ideas

メイン編

操作したいindexをactionとして渡すのは問題ない。
配列の要素を書き換えると怒られる。

stateの中身を書き換えてはいけないので、stateに配列を持たせた場合、pushやpopを使ってはならない。
必ず新しい配列を作る.
(公式) https://redux.js.org/recipes/structuring-reducers/immutable-update-patterns#inserting-and-removing-items-in-arrays
(日本語) https://qiita.com/ssnyarii/items/f295c45e1705677a7a48

一回のactionごとにstateがもっている配列の中身を減らしていって、配列が空になったときに結果を表示させたい場合、配列が空かどうかの判定は誰がやるのか -> Containerかな
Reducerが担当しても、配列が空だったときの処理を状態だけでやりきるのは無理そう

結果表示編

ボタンを押すと何種類かのビューを順繰りに切り替えられるようにするようなUIを作りたい場合、
片方向循環リストを作るよりは、indexだけを状態として持たせてしまう
reducerとしてはindexをプラス1すれば良いのだが、どこかで0に戻さなければいけない。
方法は以下の通り

  • indexをactionとして持たせる。0に戻すのはコンテナで行う
  • indexがリストの端に達した時点でindexを0に戻すactionを実行する
  • INT_MAXになったら0に戻す処理をreducerに追加する
    その際、リストの長さをReducerは知らないので、コンテナにindexを持たせる

結果の表示に関しては他に良い方法がない気がする。
せめてINT_MAXになったらリセットする機能くらい入れたい
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER

t-morisawa commented

結果表示について

ShowResult をコンテナコンポーネントにする必要性を感じない。
AppContainer でstateからデータを取ってきて、データを下ろしていくようにしてほしい。

t-morisawa commented

一旦終わりでいいかな