CODEWITHSUNDEEP
ruby-on-railsruby
neo
neo

Reputation: 4116

Super to merge hash from parent class

I have the following private method in V6 of my API:

def format_rentals(rentals)
  rentals.map do |rental|
    {
      start_time: rental.reservation_start_time.to_i,
       end_time: rental.reservation_end_time.to_i
     }
 end

On V10 of the API.

V10 inherits from V9, V9 from V8, so on...

I would like to a new attribute to the method.

is there a way to not cut and paste the whole method onto V10 and then add the new attribute.

What I don't want to do:

def format_rentals(rentals)
  rentals.map do |rental|
    {
      start_time: rental.reservation_start_time.to_i,
      end_time: rental.reservation_end_time.to_i,
      status: rental.status ### Adding this on V10

     }
 end

I would love to call super and merge to the hash, so far I've tried: 

def format_rentals(rentals)
  rentals.map do |rental|
    super.merge(status: rental.status)
   end
end

I probably don't have the syntax correct... Thanks for the help!

Upvotes: 0

Views: 2604

Answers (2)

max
max

Reputation: 102001

The problem is that you are calling super inside the block you are passing to .map. Which means that format_rentals is being called in each iteration. You could write a method which formats a single record instead:

def format_rentals(rentals)
  rentals.map { |r| format_rental(r) }
end


def format_rental(rental)
  super.merge(status: rental.status)
end

Or get creative with blocks and yield.

However might be a good time to take a step back and consider if this is a good solution - should your controller really be responsible for serialising rentals? Especially if you have different representations in different versions of the API. I would consider using ActiveModel::Serializers or jbuilder.

class V9::RentalSerializer < ActiveModel::Serializer
  attributes :start_time, :end_time

  def start_time
    object.reservation_start_time.to_i
  end

  def end_time
    object.reservation_end_time.to_i
  end
end

class V10::RentalSerializer < V9::RentalSerializer
  attributes :start_time, :end_time, :status
end

But - creating a deep class hierarchy based on the API version is hardly optimal either. The reason is that it becomes a very delicate card-house that is impossible to decipher for someone who has not followed the project from day one.

You might want to consider having a "base" which follows the current version and "adapters" which backport for legacy versions. 

class MyApi::RentalSerializerBase < ActiveModel::Serializer
  attributes :start_time, :end_time, :status

  def start_time
    object.reservation_start_time.to_i
  end

  def end_time
    object.reservation_end_time.to_i
  end
end

class V7::RentalSerializer < MyApi::RentalSerializerBase
  attributes :start_time, :end_time

  # lets say version 7 has a weird bug so that end_time is in iso 8601 instead
  # however since clients rely on the defect 
  # being there we cannot change the response
  def end_time
    object.reservation_end_time.iso8601
  end
end

class V10::RentalSerializer < MyApi::RentalSerializerBase; end

Upvotes: 2

Jordan Running
Jordan Running

Reputation: 106027

This is pretty easy if, in your superclass, you extract the contents of the block into its own format_rental method:

class V9 < V8
  def format_rental(rental)
    { start_time: rental.reservation_start_time.to_i,
      end_time: rental.reservation_end_time.to_i
    }
  end

  def format_rentals(rentals)
    rentals.map {|rental| format_rental(rental) }
  end
end

Then, in your subclass, you need only override format_rental:

class V10 < V9
  def format_rental(rental)
    super.merge(status: rental.status)
    # Or: super.tap {|r| r[:status] = rental.status }
  end
end

super will return the hash with the :start_time and :end_time keys; then you need only add your :status key and you're done. format_rentals needn't be modified at all.

Upvotes: 3

Related Questions