CODEWITHSUNDEEP
rubyduck-typing
Ryan Castillo
Ryan Castillo

Reputation: 1125

Does this look right as Ruby's duck typing?

I created a program that tracks car mileage and service history in order to update the user for upcoming service needs for the car.

I have three classes: Car, CarHistory, and CarServiceHistoryEntry. The third one is straightforward; it holds all the attributes associated with a service: date, mileage, service performed, etc. The CarHistory class is as follows:

require_relative 'car_service_history_entry'

class CarHistory
  attr_reader :entries
  def initialize (*entry)
    if entry.size > 1
      @entries = []
    else
      @entries = entry
    end
  end
  def add_service_entry entry
    @entries << entry
  end
  def to_s
    entries_string = ""
    @entries.each {|entry| entries_string << "#{entry.to_s}\n"}
    entries_string
  end
end
  1. In initialize, should the class of entry be checked?
  2. In add_service_entry, adopting duck typing (as in Andy Thomas's argument in "Programming Ruby"), would I even test if a CarServiceHistoryEntry could be added? Couldn't I just pass a String instead of setting up and then adding CarServiceHistoryEntry in my unit testing?
  3. Since the only necessary attributes of a CarHistory are the entries array and the to_s method, should I just scrap this class all together and put it into the car class?

Upvotes: 1

Views: 304

Answers (3)

Gishu
Gishu

Reputation: 136633

For 1 and 2, you need to release your tight grip on "strict-typing" when you move to a loose-typed language like Ruby.

  • Should you check your input arguments ? The traditional answer would be yes. An alternative way would be to have good names and unit tests that document and specify how the type is supposed to work. If it works with other types, fine.. that's an added bonus. So if you pass in an incompatible type, it would blow up with an exception, which is good enough in most-cases. Try it out and see how it feels (possible outcomes: Liberating / "Retreat!". But give it a fair try.). Exceptions would be if you're designing public APIs for shared libraries - in which the rules are different. You need to fail fast and informatively for bad-input.
  • As for clubbing car_history into car - I'd ask what the responsibilities of your Car class are. If maintaining its own history is one of them, you could club them. In the future, if you find a lot of methods creeping in related to car history, you could again reverse this decision and extract the CarHistory type again. Use the SingleResponsibilityPrinciple to make an informed decision. This is just OOP - Ruby doesn't degrade object design.

Code Snippet: the code can be more concise

# just for simplicity, I'm making HistoryEntry a string, it could be a custom type too
class CarServiceHistoryEntry << String
end

class CarHistory
  attr_reader :entries
  def initialize(*history_entries)
    @entries = history_entries
  end

  def add_service_entry(entry)
    @entries << entry
  end
  def to_s
    @entries.join("\n")
  end
end

irb>x = CarHistory.new("May 01 Overhaul", "May 30 minor repairs")
irb>x.add_service_entry("June 12 Cracked windshield")
irb>x.to_s
=> "May 01 Overhaul\nMay 30 minor repairs\nJune 12 Cracked windshield"

Upvotes: 1

DigitalRoss
DigitalRoss

Reputation: 146123

It's hard to comment on the relationship of the CarHistory class to your others, but I'm sure it will become clear to you as you go along.

A couple of your methods could be simplified, although I must say I didn't understand the if in initialize, perhaps it was just backwards and should have been > 0.

def initialize *entry
  @entries = entry # if not specified it will be [] anyway
end

def to_s
  @entries.join "\n"
end

And yes, Ruby should be simple. You don't need to litter your code with runtime type checks. If the code runs your unit tests then you can just declare victory. The zillions of explicit conversions tend to patch up type errors anyway.

Ruby is going to check your types at run-time anyway. It's perfectly reasonable to leave the type checking to the interpreter and put your effort into functional tests.

Upvotes: 1

Brian61
Brian61

Reputation: 59

I'll skip the first two questions and answer the third. If the only attribute of a CarServiceHistoryEntry is a string, then yes, scrap CarHistory (as well as CarServiceHistoryEntry) and add a service_history attribute to Car which would just be an array of strings. Until proven otherwise, simpler is better.

As to duck typing, you would never want to test if something 'is a' only see if it 'responds to' (at most).

Finally, to answer question #1, no its supposed to be even simpler :)

Hope this helps, Brian

Upvotes: 0

Related Questions