# Ensuring a non-zero radius when constructing a circle

Is there a little "better" way of writing the line:

raise RuntimeError if not radius.is_a? Numeric or radius == 0


I want to only allow numbers bigger than 0.

class Shape
def initialize(name)
@name = name
end

def to_s
"name: #{@name}"
end
end

class Circle < Shape
super(name)
end

def area
end

def to_s
end
end


This is my spec:

describe Circle do
it "should raise exception if radius is 0 or nil" do
expect {
Circle.new("0", 0)
}.to raise_error(RuntimeError)

expect {
Circle.new("nil", nil)
}.to raise_error(RuntimeError)
end
end


Maybe this?

raise RuntimeError unless radius.is_a?(Numeric) && radius != 0

• What if the radius is negative? Is that acceptable? – 200_success Nov 29 '17 at 0:19
• Why do you have a default radius = 0 if that is an unacceptable value? – 200_success Nov 29 '17 at 0:20

As mentioned in the comments by 200_success:

• Why is zero the default, if it's invalid?

Conceptually, a circle with a zero or negative radius might not make a lot of sense, but there's nothing in your code that would break because of it. The area will never be negative since the radius is multiplied by itself. The area might be zero, but again, that is the area of a zero-radius circle, so strictly speaking it's still valid.

You're also checking the type of the radius argument. You can argue either way if that's a good idea. It just raises a RuntimeError, with no extra explanation of what went wrong. If the check wasn't there, and you called, say, Circle.new("banana", "banana").area, you'd just get a NoMethodError instead. Not saying that's better, just saying that it's much worse either. Of course the current check happens in the constructor, which might be nice, but is it necessary?

For most cases, I lean toward saying "Garbage in, garbage out". If you've set the radius to "banana", then don't expect the math to make sense. Since Ruby isn't strictly typed and there's no compile step, the exceptions will occur at runtime anyway. So even if you try to add a lot of type-checking, it's debatable how much value it adds. Especially in this case, since raising a RuntimeError with no extra explanation seems less useful than letting, say, a NoMethodError occur, which will at least tell you things like undefined method**' for "banana".

Regardless, no reason to be dogmatic either way; type-checking definitely has its uses, and I'm not saying to always avoid it. Just make sure it adds some value.

Besides, it'd be better to raise an TypeError, since that's a lot more specific to this case than a general things-just-kinda-broke-RuntimeError.

I'd still argue that radius can indeed be zero, and that's fine. It's a strange circle to contemplate, but it's not impossible.

Again, I'd probably just skip the checking entirely, but if I were to add it, I'd write

raise TypeError.new("Radius must be numeric") unless radius.is_a?(Numeric)


And if you do want to avoid strange radii, you can add a separate check after that:

raise ArgumentError.new("Radius must be > 0") if radius <= 0


You could simply write:

raise ArgumentError.new("Radius must be > 0") if radius == 0 if radius.to_f <= 0


I would consider it bad style to raise an exception without a message. Most Ruby developers would also use ! and && instead of not and and`