1
\$\begingroup\$

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 << 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
\$\endgroup\$
0
\$\begingroup\$

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.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ That's helpful. Regarding successor, the difference between what I've done and what Metz did was I had to pass in successor_number rather than use a straight up successor object. Good point about the order of arguments for #new. Inheritance does seem like a better fit in this case. I did it this way as an exercise. I'll need to reread POOD to figure out when composition is actually a good idea. \$\endgroup\$ – Mickey Sheu May 1 '17 at 19:09
  • \$\begingroup\$ @MickeySheu github.com/sandimetz/99bottles/blob/… Metz does exactly what you did? have a look at the successor method? \$\endgroup\$ – BKSpurgeon May 1 '17 at 22:25
  • \$\begingroup\$ I tried to, but I don't have enough reputation yet! \$\endgroup\$ – Mickey Sheu May 3 '17 at 14:28
  • \$\begingroup\$ And approving answer doesn't give me enough to upvote either. awells \$\endgroup\$ – Mickey Sheu May 3 '17 at 14:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.