Print lyrics of 99 Bottles of Beer I read 99 Bottles of OOP, and one of the offhand comments was that doing the 99 bottles problem with composition was another route that one could take (the book used inheritance).  Here is my attempt.  Here are the lyrics needed: Beer Song

Some difficulties I had: 

1) Implementation of successor was tricky.  I could not simply pass in a successor object because then BottleNumber(99) needed to hold BottleNumber(98) which needed to hold... Instead I used successor_number and generated a successor when needed.  

2) Factory seemed messy - the arguments for the initialize method stacked up and up.  Named arguments only made things longer. Sometimes I had to implement a default_object and other times I could use default named parameters.  Should this be standardized throughout?  

Comments welcome

class BeerSong

  def verse(number)
    bottle_number = BottleNumber.for(number)
    "#{bottle_number} of beer on the wall, #{bottle_number} of beer.\n".capitalize +
    "#{bottle_number.action}, #{bottle_number.successor} of beer on the wall.\n"
  end

  def verses(starting,ending)
    starting.downto(ending).map do |number|
      verse(number)
    end.join("\n")
  end

  def song
    verses(99,0)
  end
end

class BottleNumber
  attr_reader :number, :container, :pronoun, :quantity, :action, :successor_number

  class &lt;&lt; self
    def for(number)
      return number if number.is_a? BottleNumber
      case number
      when 0
        BottleNumber.new(number, quantity: 'no more', successor_number: 99, action: 'Go to the store and buy some more')
      when 1
        BottleNumber.new(number, container: 'bottle', pronoun: 'it')
      else
        BottleNumber.new(number)
      end
    end
  end

  def initialize(number, container: 'bottles', pronoun: 'one', quantity: nil, action: nil, successor_number: nil)
    @number = number
    @container = container
    @pronoun = pronoun
    @quantity = quantity || default_quanity
    @action = action || default_action
    @successor_number = successor_number || default_successor_number
  end

  def to_s
    "#{quantity} #{container}"
  end

  def default_successor_number
    number - 1
  end

  def default_quanity
    number.to_s
  end

  def default_action
    "Take #{pronoun} down and pass it around"
  end

  def successor
    BottleNumber.for(successor_number)
  end
end
Some comments when looking through your code:


Successor


I think that's fine - how else were you going to do it? Metz does exactly the same thing doesn't she? When successor is called a new bottle number - 1 is created - unless of course the bottle number is zero, in which case you start right back at 100.


Knowledge of the arguments and their order:


Consider this:

BottleNumber.new(number, quantity: 'no more', successor_number: 99, action: 'Go to the store and buy some more')


I don't like this. Why? Because every time you need to instantiante a bottle you need to KNOW what goes in there and you also need to know the order in which the arguments go in. You could eliminate the need to know the argument order by passing in a hash. That's probably the only bit of criticism i can add.


Inheritance


for this particular problem, inheritance seems like a better fit. it just seems a lot cleaner than dealing with the messiness of passing in those parameters.

anyways those are just my thoughts and i hope you find them of some use.