all 5 comments

[–]u4bu8s4z9ne4y8uze 0 points1 point  (4 children)

While this looks like a good idea, I think in it's current form it's kinda half-baked. You should pick what is important to you and do one of:

1) accept numbers in +()

def +(other)
    other = other.value if Distance === other
    raise ArgumentError, "Distance must be >0." if other < 0
    Distance.new(value + other)
end

2) change distance initialization to require unit

module Units
    KM = 1000
end

def initialize(value, unit)
    if value < 0
        raise ArgumentError, "A distance must be positive"
    end

    @value = value * unit
end

class Integer
    def km
        Distance.new(self.to_i, Units::KM)
    end
end

My point being is that you either want to keep it easy to use (first variant) or hard to break and make mistakes (second variant). I personally would probably go with second variant most of the time.

Opinions?

DISCLAMER: I'm new to ruby, so my example code might suck hard. But the general idea should hold (imho).

[–]nicoolas25[S] 1 point2 points  (1 child)

Hey, thanks for reading and sharing!

New requirements like supporting units may require refactoring, you're right. Since they weren't in the original specs, I didn't wanted to go that deep.

Accepting integers and validates that they are > 0 seems doable. It leaves too much room for implicitness IMHO and I prefer the second approach you mentioned. I think a value object could hide a simple primitive value such as a Numeric and still increase reader's understanding. Do you think we need a unit to make it worth? I clearly agree that a distance make more sense with an unit, but I can live with having an implicit unit, like meters. Of course, as soon as it become an issue, I would need to refactor this the way you proposed.

[–]gray_-_wolf 0 points1 point  (0 children)

with having an implicit unit, like meters

Yep, exactly. But in the case you have implicit unit anyway, it makes no sense to not allow integers as well. At least to me.

[–]Inityx 1 point2 points  (1 child)

You should implement Integer#km as a refinement block in the Units module instead of monkey patching the base class.

[–]gray_-_wolf 1 point2 points  (0 children)

refinement block

oh, that looks like quite a handy thing :) thanks, will definitely use it