ikegamir-impressions

[ikegamir-sw]

お疲れさまでした！ちゃんと動きそうですね:clap: ただし、正直言って実装はかなり微妙です。。ちょっとずつおべんつよしていきましょう。

指摘

コード上すぐ直せること

• いかなる理由があろうともvarを使うのはやめましょう。基本的にconst、どうしても再代入が必要なケースではletを使ってください。 https://github.com/sw-otay/app-development-training/blob/4eaaea8077b9d54731640067f8deccf28bf8ac11/src/App.vue#L40-L41

• この箇所にrefを使うのは不適切です。普通の配列でOKです。 https://github.com/sw-otay/app-development-training/blob/4eaaea8077b9d54731640067f8deccf28bf8ac11/src/App.vue#L3-L34

  • refとは……
  • ミュータブル(=変更可能)
    • おみくじの中身をアプリの使用中に書き換えますか？
  • リアクティブ(=相互反映)
    • おみくじの中身をアプリから書き換えますか？
    • おみくじの中身をアプリの使用中に書き換えて、それを画面に反映しますか？

• おみくじの結果のIDを入れる変数だと思いますが、 count ではなくないですか？ https://github.com/sw-otay/app-development-training/blob/4eaaea8077b9d54731640067f8deccf28bf8ac11/src/App.vue#L36

  • 適切な名前にしましょう

• ここのロジックがむずかしい書かれ方してますね https://github.com/sw-otay/app-development-training/blob/4eaaea8077b9d54731640067f8deccf28bf8ac11/src/App.vue#L42

  • max + 1 - min って1足して1引いてるから意味ないのでは？
  • maxの定義は不要では？
  • 配列の長さ( items.length )と同じですよね？将来おみくじの結果増やしたらこれもいちいち書き換えることになりますけど、面倒じゃないですか？
    • 「浅草寺のおみくじには半吉と末吉があるので、追加してください」って言われることを考えてみましょう
    • なんでお寺なのに「神籤」があるんですかね。

設計の話

結果表示のアプローチについて

これで動くは動くんですが、かなり悪手です。 ということで、別の手段も考えてみましょう。複数浮かべば浮かぶだけ。issueにどういう方針で実現するか、日本語でコメントしてください。 https://github.com/sw-otay/app-development-training/blob/4eaaea8077b9d54731640067f8deccf28bf8ac11/src/App.vue#L48-L53

太田さんがとったアプローチ

• template上で
• おみくじの結果一覧をループして、選出したIDと一致していたら表示する

  微妙な点

  ロジックが画面表示側に漏れている

  「おみくじの結果（のオブジェクト）」はロジックが導き出すべきもの(= <script/> の中で作り出されるべきもの)です。

  {
  id: 5,
  name: '凶',
  description:
    '凶です'
  }

  これを今template側にロジックを書いてやらせています。 これでも動きますけど、ひとつの目的のことは複数箇所でやるより単一箇所でやったほうがよさそうですよね？

再利用性が低い

「おみくじの結果の表示は子コンポーネントに任せたいので、結果を子コンポーネントに渡してください」というケースを考えると、

  {
    id: 4,
    name: '吉',
    description:
      '吉です'
  }

を子コンポーネントに渡したいけど、今の書き方だと大幅改修が必要です。

不要な全件ループが行われている

今は5つしかないから全然大丈夫なんですが、もし結果が100万件とかある場合、毎回100万回ループすることになります。 シュッと書くと自然とスケーラブル(数字がでかくなってもちゃんと動く)になります。

プログラムに直接関係ない細かいこと

• おみくじの結果って見出しなんですかね……？

  • わからん。見出しかもしれない。 https://github.com/sw-otay/app-development-training/blob/4eaaea8077b9d54731640067f8deccf28bf8ac11/src/App.vue#L50

• （道具としての話）↓みたいに出されるの、わたしがユーザーだったら「見りゃわかるわ！」って思います。

  大吉
  大吉です

  • 受託開発で、お客さんがこういう表示にしてくれーって要求してきたら「くどいなぁ」ってなると思います。

[InoueTakahiro-sw]

result.value = Math.floor( Math.random() * (items.length + min));

これに修正してもらってますが、これだと切り替わらない（消える）ことがある気がします。 https://github.com/sw-otay/app-development-training/commit/513dc1791981f1ddcf2fd903774bc49bbfca7f0f#diff-7a7a37b12ee1265d7e27577ec286120d2cbc0940908635e264a2be44ccb9a8a0R41

[ikegamir-sw]

ということは、バグってますよね。 https://github.com/sw-otay/app-development-training/blob/513dc1791981f1ddcf2fd903774bc49bbfca7f0f/src/App.vue#L41 このコードで何が起こるか説明できますか？できれば直せると思います。

あと、この課題ではそんなに重要ではないんですが、コミットメッセージは「自分がコードに対して何をしたか」を書いたほうがいいです。他人が見るものだからです。 「指摘事項修正」は最悪のコミットメッセージのひとつです。他人には指摘事項がわからないからです。

[sw-otay]

result.value = Math.floor( Math.random() * (items.length + min));

のバグを修正しました。 0〜1の不動小数点を生成するMath.random()に最大値をかけて最大値の数分の数値の中からランダム値を生成する、minを足すのは0〜でなく最小値〜とするためだったのですが ()の位置がおかしくなってしまっていたためバグっていました。

[sw-otay]

結果表示のアプローチの改善案

• Githubissues.
• Githubissues is a development platform for aggregating issues.