質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

88.35%

ネストが深いリレーションの為、eachのネストが発生するスクリプト(本文中にあり)がでます、こういうのは設計を見直すべきなのでしょうか?

受付中

回答 3

投稿

  • 評価
  • クリップ 0
  • VIEW 2,341

qaz3330

score 113

Userに紐づく、店舗の従業員の給与合計を出力するスクリプトを書きたいです。

以下のようなのを作成したのですが、each がネストしているのがすごく違和感を感じます。

私自身の経験が浅いための勘違いかもしれませんが、
eachがネストしている事自体はさほど問題ないのでしょうか?

sum = 0
User.first.shops.each do |shop|
  shop.employments.each do |employment|
    sum += employment.price * employment.hour
  end
end

今後、このデータの数も増えてくるため、
今のうちに設計を見直すべきなのかと思っております。

ご助言頂けると幸いです。
よろしくお願いします。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

0

私だったら、それぞれのモデルクラスに集計用のメソッドを書いて、それを内部で呼び出す形式にします。
で、後々パフォーマンス等々の問題が出たときには、それぞれのメソッドでがさっと取ってくるように変更してゆきます。
まあ、中間のメソッドが必要ないのであれば、最上位のuserに一括でemploymentを取得して集計するようにしますが。

class User < ActiveRecord::Base
  has_many :shops
  # 普通に使うとき
  def total_payment
    payment = 0
    self.shops.each {|s| payment += s.total_payment }
    return payment
  end

  # 高速化してみる
  def hi_speed_total_payment
    user_table = User.arel_table
    shop_table = Shop.arel_table
    emp_table = Employment.arel_table
    join_s = emp_table.join(shop_table, Arel::Nodes::OuterJoin).on(emp_table[:shop_id].eq(shop_table[:id]).join(user_table, Arel::Nodes::OuterJoin).on(shop_table[:user_id].eq(user_table[:id]).join_source
    total = 0
    Employment.joins(join_s).where(user_table[:id].eq(self.id).each do |emp|
      total += emp.total_payment
    end
    return total
  end
end

class Shop < ActiveRecord::Base
  belongs_to :user
  has_many :employments

  def total_payment
    payment = 0
    self.employments.each {|emp| payment += emp.total_payment }
    return payment
  end
end

class Employment < ActiveRecord::Base
  belongs_to :shop

  def total_payment
    return self.price * self.hour
  end
end


Railsを使っていて、パフォーマンスの問題が出るのは、ネストの深さというよりは、DBアクセスのオーバーヘッドにボトルネックがあるので、(eachで回してると、それぞれのeachの中でDBアクセスが発生する)パフォーマンスの問題を考えるなら、どこでDBアクセスが発生しうるのかを考えるのがポイントかと。
eager_loadingとか使ってて、あらかじめ関連インスタンスをメモリにロードしてる場合はループで回しても同じとか、その辺を考える分けです。

可読性の問題とか、その辺を考えるなら、モデルクラスのメソッドをコールして、その内部でループを回す方法が良いかと思います。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2016/05/18 17:42

    ご回答ありがとうございます!

    可読性の面やパフォーマンスの面からアドバイスを頂き、大変勉強になりました。

    回答を読んでいる中高速化について質問があります。

    表現があってるかわかりませんが、
    Controllerのところで、includeして、viewに渡します。
    (質問文にそこまで書くべきでした・・・。すみません。)

    @users = User.include(:shops, : employments)


    このControllerのところでincludeしておけば、高速化したhi_speed_total_paymentメソッドを使用する必要がないのでしょうか?

    気になる点としましては、Controllerでincludeしてさらにhi_speed_total_paymentをつかうと、高速化のつもりがさらに遅くなるのかなという点です。

    Controllerでincludeするため、total_paymentメソッドを使用するのが今回のようなケースではあっているのかなと素人ながらに思ったのですが、いかがでしょうか?

    キャンセル

  • 2016/05/18 18:29

    おっしゃるとおり、Controllerでincludeしていれば、サンプルの高速化は意味が無いですね。
    ちょっと詳しく調べてないので、あやふやですが、Arelが採用された後のRaills(確か3.0以降)は、.eachとか、.allとか、.countなどのメソッドがコールされるまでDBアクセスは発生しません。
    で、あらかじめincludesをしてあれば、モデルのメソッドで.eachがコールされたときに、関連モデルも含めて一気に取ってくる仕様になっているはずです。
    なので、あらかじめincludesしてあるのであれば、通常のメソッドを使っても大丈夫なはず。
    なお、高速化の方は通常の.eachアクセスとは全く別にDBアクセスを発行するので、どっちにしても変わらない、という事になります。(高速化のメソッドコールで余計に1回DBアクセスが走るかな)
    実際にDBアクセスが発生するかは、ログを読んで確認するのが一番手っ取り早いかと思います。

    キャンセル

  • 2016/05/18 23:25

    ありがとうございます。
    DBの高速化などの経験がこれまであまりなかったので、大変参考になりました。
    ログを読んで確認するというのに、尽きるのですね。
    頑張ってログをみて、高速化対応やってみます。

    キャンセル

0

SQLで条件を定義するのが困難な場合を除いて、集計はDB側でやるのがセオリーです。集計のためにアプリケーション(Rails)側でループを回すのは典型的なアンチパターンです。

モデルの構造がわからないので想像ですが、

User.first.shops.select('SUM(employments.price * employments.hour) AS total').joins(:employments).first.total


こんな感じで行けるんじゃないでしょうか。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2016/05/18 23:24

    こういうのはアンチパターンなんですね。
    勉強になりました。

    店舗の従業員の給与合計という風に記載しましたが、実際に運用するのは、もう少し違っていて、合計の金額が数分毎に変わります。

    そのような中、今回の仕様上、できれば、ユーザーがアクセスした際に、そのユーザーに紐づく合計金額を出したいです。

    もし上記要件を満たそうとした場合に私が考えたのは

    ・5分毎にDB側で集計をするバッチ処理を書く(5分かどうかは未定ですが)
    ・今回の使用を考慮し、includeなどでeager_loadingさせる方法で対応させる

    のがいいのかなと思いました。

    やはりこういう場合ですと、バッチ処理のほうがいいのでしょうか?

    キャンセル

  • 2016/05/19 09:34

    随時計算するかバッチにするかは状況次第ですので、どちらが良い悪いはありません。たとえば、全結果の大半が利用されるのか少数しか利用されないのか、などで判断します

    キャンセル

-1

他の方がきっとしてくれるであろう回答が勉強になりそうで楽しみです。

私としてはネストの深さより、スクロースせずに見える範囲にループが収まるか否か、という方を重要視する場合が多いと思います。ループが 3 重くらいでも素直な構造のもの (たとえば x, y, z 軸についてのループとか) でループ全体が 20 行で収まっているなら特におかしいとは思いません。実際にはネストの深さというか、条件分岐なども含んだインデントの深さで考えますが。

他のところの処理にもよりますが、今回のようなケースで、クラスを用いてさらにコードのネストを減らすことは有用なことも少なくないと思います。ネストを減らすことが目的ではなく、コードの見通しをよくし、仕様変更などに強くするのが目的になりますが。
具体的には、Shop クラスを作って totalSalary という getter を用意し、Employment クラスを作って salary という getter を用意する、という感じでしょうか。たとえば、支店ごとの合計の他に、支店内の部署ごとの合計も出したい、とかなったときに Employment クラスには手を入れなくても対応できるようになります。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 88.35%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

同じタグがついた質問を見る

  • トップ
  • Rubyに関する質問
  • ネストが深いリレーションの為、eachのネストが発生するスクリプト(本文中にあり)がでます、こういうのは設計を見直すべきなのでしょうか?