grouper variation

class GroupersController < ApplicationController::Base
  def create
    @grouper = Grouper.new(leader: current_member)

    if @grouper.save
      ConfirmedGrouperEmails.new(@grouper).deliver
      AssignBarForGrouper.enqueue(@grouper.id)

      redirect_to home_path
    else
      render :new
    end
  end
end


# app/mailers/confirmed_grouper_emails.rb
class ConfirmedGrouperEmails
  def initialize(grouper)
    @grouper = grouper
  end
  
  def deliver
    LeaderMailer.grouper_confirmed(member: @grouper.leader.id).deliver
    WingMailer.grouper_confirmed(wings: @grouper.wings.pluck(:id)).deliver
    AdminMailer.grouper_confirmed(grouper: @grouper.admin.id).deliver
  end
end

The advantage is quite simple: At some point there might have to be some kind of state – this is where you'd then either convert all consumers of your ConfirmedGrouperEmails API to use the instantiated version or provide a kind of wrapper to have the class method create an instance and deliver it by calling ConfirmedGrouperEmails.deliver(grouper). This is not premature optimization but a real issue.

Simply put: Don't use class methods unless you really have to. Ruby is an OOP language so use OOP.

In this specific example you're right, there's no improvement. But if you were to add other methods beyond deliver, which also make use of @grouper, then you get the improvement of not having to pass the parameter around constantly. It's not really any different than preferring "hello".upcase to something like uppercase("hello")

@tel I think the improvement is readability: .deliver(grouper) comes across as delivering a grouper, while .deliver is read as simply delivering whatever it was called on.

Class methods resist refactoring.

@shime 👍

Put another way, passing the argument to a class method is not "exemplary" (a la Sandi Metz). So future development efforts (often by other devs) are likely harmed, since this pattern resists refactoring.