## 8章 8.2(P.154~177) * 任意参加 * ファシリテーター: 照屋 ### 話す・書く順番決めルーレット in Ruby ```ruby ['kanno', 'shioi','hsu', 'chen', 'teruya', 'kanazawa', 'okura'].shuffle.each { |v| puts ">[name=#{v}]\n\n" } ``` in JavaScript ```javascript const l = ['kanno', 'shioi', 'hsu', 'chen', 'teruya', 'kanazawa', 'okura'] l[Math.floor(Math.random()*l.length)] ``` ### ファシリテーター決めルーレット in Ruby ```ruby %w(kawasaki shioi hsu chen teruya kanazawa okura).sample ``` in JavaScript ```javascript ['kawasaki', 'shioi', 'hsu', 'chen', 'teruya', 'kanazawa', 'okura'].sort(()=> Math.random() - 0.5) ``` ### 参加者がやること * 事前に決められた章を読む * 学んだことやわからなかったことを書き込む(任意) 当日 * 最初に感想や質問を書く時間を設ける(10分) * 時間枠 60分 * 延長枠 30分 * 例:6人参加なら、一人あたり話す時間は10〜15分 ### 勉強会時にやること ``` 各自学びや気付き、疑問を発表していく ファシリテーターがよしなに議論を進める 終了前に勉強会を振り返る?(KPTなどで)← 60分のときに仮締め ``` ### 当日ファシリテーターがやること * [ ] 次回のファシリテーターを決める * [ ] 次回までに読む範囲を決める * [ ] 次回の kibela と hackmd を用意する ## 各自が書き込む場所 >[name=shioi] > P157 スーパークラス依存による密結合を避けるため、**継承より委譲**(**委譲**)が推奨 されます。委譲とは**コンポジション構造**にすることです。 最近「オブジェクト指向のこころ」を読み直していたのですが、「継承よりコンポジション」はJavaの文脈だとよく利用されていそうです。 Rubyだと「継承よりmodule」になりそう(方向性は違うけどクラス構造以外の概念を使って共通化を制御するという目的は同じ) > P163 Utilクラス 前にも書いたけど「~Managementクラス」「Utilクラス」と命名したいクラス構造が発生した時点で何かがおかしいので再考するべき(OOPにおいて「雑多な処理を一つにまとめるクラス」は存在できない) > 8.2.4 private メソッドだらけ 言いたいことは「責務の異なるメソッドは、別々のクラスに分離しましょう 」なのだけど、クラスの成長に従ってクラスを分離するタイミングがどこかでやってくるとして、そのタイミングがいつなのかと、どうクラスを分離するかを判断するのが難しいというのが本質だと思います。 「private メソッドだらけ」というタイトルは不適切かも。 関数を意味のある単位で分解していくと数が増えるのは当たり前なので、privateメソッドが多いことが問題というわけではない。 https://github.com/Catal/Nozomi/blob/master/app/models/shift_offer.rb https://github.com/Catal/Nozomi/blob/master/app/models/shift_offer_response.rb > 8.2.8 トランザクションスクリプトパターン これがトランザクションスクリプトパターンなのかそうでないのか、の判断って難しくない…?? この次の節に「あらゆる責務の ロジックが、乱雑に絡み合う」という表現があるのですが、例えばトランザクショナルな処理が発生することを前提にしているServiceクラスにおいて「その責務とは何か」、という視点は常に発生する >[name=okura] ### 8.2.1 継承に絡む密結合 出たな継承!!! > また、スーパークラス側は、サブクラスを気にせず変更されていきます。 そんなことはないのでは?少なくとも、継承される前提のクラスはそう簡単に互換性を壊すような変更を入れないのが普通。 > discountChargeのオーバーライド実装には、基底クラス側のgetDiscountedPriceのロジックを知っていなければなりません。関連知識が1つのクラスに凝集しておらず、基底と継承それぞれに分散しており良い設計とは言えません。 これは単なるテンプレートメソッドパターンなので、さすがに言い過ぎ。親と子が協力してロジックを完成させている場合、子が親のどのメソッドをオーバーライドするべきか、という知識は子が持つべき知識であり、分散しているとは言えない。 ### 8.2.2 インスタンス変数ごとにクラス分割可能なロジック Utilクラス的なものはたいていどこにでもあるけど、ドメインに関連するコードでなければ問題ない(例えば時刻の表示を調整するだけのコードとか)。 ### 8.2.3 なんでもpublicで密結合 Rubyはデフォルトで`public`だけど、いつも`private`を書くからあまり問題だと思ったことはない。 Rubyにはパッケージの概念がなくて、不便だと思いつつも適切なドキュメントなどで注意書きはできるし、まあ大丈夫かなと思っている(例えばgemの内部でだけ使いたい定数があるときとかになかなか実現が難しい)。 ### 8.2.4 privateメソッドだらけ 実際にはそれほど困らないけど、privateメソッドが多すぎるときはリファクタリングしたほうがいいシグナルだとは思っている。 https://github.com/okuramasafumi/alba/pull/222 ### 8.2.5 高凝集の誤解から来る密結合 これはドメインによるかなあ。 ### 8.2.6 スマートUI 初耳。 ### 8.2.7 巨大データクラス Railsの仕組みに乗っているとこういうクラスは書かずに済むので便利。 ### 8.2.8 トランザクションスクリプトパターン 単にダラダラしたコードはトランザクションスクリプトパターンとは呼ばない気がする…手続き型とも違う気がする… https://speakerdeck.com/yasaichi/directions-for-the-next-generation-of-ruby-on-rails-from-the-viewpoint-of-its-active-record ここにも出てくる。 ### 8.2.9 神クラス 昔`user.rb`が2000行くらいあってアプリのロジックはだいたいここに書かれていた、ということがあった。 ### 8.2.10 密結合クラスの対処法 この本全体に言えることではあるけど、早期returnでは密結合の問題は解決しないのでは…(密結合は設計の話であり早期returnはコードの話なので) --- >[name=kanazawa] > 継承はよっぽど注意して扱わないと危険、継承は推奨しませんというの が本書のスタンスです。 これまでの輪読会でも散々出てきた。チームでもこれが共通認識になっている感。 Rubyで委譲機能を提供する標準ライブラリ(`forwardable`)があった。 [標準添付ライブラリ紹介 【第 6 回】 委譲](https://magazine.rubyist.net/articles/0012/0012-BundledLibraries.html) 関係ないけどRubyist Magazineの標準ライブラリ紹介シリーズが面白そうなので今度読んでみようと思った。 > private メソッドだらけ > 筆者の経験上、private メソッドが多いクラスは、単一責任ではなく、 多くの責務を持ってしまっています privateメソッドが増えてきたときは責務を分割できる可能性 > 8.2.6 スマート UI > 開発初期、サービスをとにかく急いでローンチさせるため、複雑な金額計算ロジックや分岐ロジックがフロント側に実装されてしまいがち そうなんだ? むしろそっちのが難しいと思うけど。 > トランザクションスクリプトパターン キャタルのバックエンドでいえばサービスクラスにトランザクションが書かれることが多い印象です。 https://github.com/Catal/Rewrites/blob/master/app/services/summaries/delete_feedback_service.rb#L1-L24 (ちょっと野心的な雰囲気が出ているが…) この本のRuby版が読みたいと思った。やはりJavaの特定の機能を使って問題を解消してるところが多いので。 [Amazon \| Design Patterns in Ruby \(Addison\-Wesley Professional Ruby Series\) \| Olsen, Russ \| Ruby](https://www.amazon.co.jp/Design-Patterns-Ruby-Addison-Wesley-Professional/dp/0321490452) --- >[name=kanno] p.155 >継承はよっぽど注意して扱わないと危険、(この本では)推奨しない - 委譲の考え方がよくわかっていません… - 丸ごと機能をもらうというより、一部使いたいところを取ってこれる、みたいなイメージを持っています > 委譲では、再利用したい機能を自分に取り込むのではなく、その機能を持つオブジェクトに処理を依頼します。 https://magazine.rubyist.net/articles/0012/0012-BundledLibraries.html わかりやすい説明があったので👀 by Kanazawa P.170 > 初学者向けの本にはpublicが標準扱いで出てくる - 確かに、最初はpublicにメソッドを置くのが普通なのだと思っていた…。privateメソッドを使うときも「これはどこのクラスからも呼ばれてないからprivateにしておこう」みたいな考え方をしていた - 本来であれば、設計上の意図(or 強い意志?)によってprivateにするメソッドを決定するべきなのかなと思った --- >[name=chen] #### 8.8 武闘家の物理攻撃クラス(継承版) >もともとdoubleAttackDamageは、singleAttackDamageを実行せず独自にダメージ値を計算するロジックでした。ところが、singleAttackDamageを実行するロジックへ変更した結果、FighterPhysicalAttack側でオーバーライドしたsingleAttackDamageメソッドが2回呼ばれるようになりました。FighterPhysicalAttack.singleAttackDamage内の20が2回加算されることにより、仕様と異なるダメージ値になってしまったのです。 これは、単純に変更した不具合ではないんですか… 継承などには主な問題点ではないと思ってます:thinking_face: #### 8.16 基底メソッドを完全に上書きしたオーバーライド > ある継承クラスにとっては関係があっても、別の継承クラスにとっては無関係なメソッドが登場しはじめると問題です。どこからどこまで関連があるのか、ロジックの追跡が非常に困難になり、デバッグや仕様変更でつらい思いを味わうことになります。 これはoverrideの利用する問題な気がしますね。 継承する時親クラスのメソットを利用するのが一般的ですが、 overrideを使うと継承されたロジックをやり直すことですので、 ロジックの整理や追跡、注意しないといけないかなと思います。 (overrideを書く時これは基本的だと思いますwww) #### 8.2.2 インスタンス変数ごとにクラス分割可能なロジック 例のコードはやばいんじゃないですかww w 単一責任原則を全然守ってないため… だが、Utilの中に一体何のメソッドをまとめたほうが良いのか もむずいと思います (逆に分類しにくいメソットはどこに置いとけば良いかと) #### 疎結合高凝集 > 高凝集を意図して強く関係していそうなロジックを一箇所にまとめ上げようとしたものの、結果として密結合に陥っているケースは非常に多く見られます。誰もが極めて陥りやすい罠です。 それぞれの概念は分離し、疎結合にしなければなりません。そのため、設計においては`疎結合高凝集`と呼称し、セットで語られることが多いです。 それはそうだです。 が、この概念を実践するのが簡単ではない気がしますね…:thinking_face::thinking_face::thinking_face: --- >[name=teruya] > FighterPhysicalAttack側でオーバーライドした singleAttackDamageメソッドが 2 回呼ばれるようになりました。(p.156) 継承が要注意なことは散々学んだけど、このバグの踏み方は想定してなかった(普通にやっちゃいそう) > 武闘家の物理攻撃クラス(コンポジション版)(p.157) moduleに切り出してPhysicalAttackとFighterPhysicalAttackで使うのを考えちゃうかも > sellingPriceの分解(p.174) テーブルとモデルを結びつけて考えると密結合を避けられそう ## 振り返り > [name=shioi] コードは高凝集・疎結合になっていて欲しいのになかなかそうならないのは、サービスと共にコードは成長するし成長するということはコンテキストが増えていくということだから… > kanazawa - 継承よりも委譲 - Rails wayに乗ろうぜぇ!!! > [name=okura] 今日は割と厳しめに突っ込みを入れましたが、それも自分の役割かと思っています >[name=kanno] - 委譲を使いこなせるようになりたい >[name=chen] 単一原則が重要だけど、疎結合高凝集の実践が難しい:cry: >[name=teruya] コードは徐々に増えていくのでどの段階で切り出すかとかはめちゃくちゃ難しいですね… ### ⁠次回(9/16) 読む範囲:第9章 179~201 ファシリテーター: kanazawa