Ensuring a non-zero radius when constructing a circle

Question

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
  def initialize(name, radius = 0)
    raise RuntimeError if not radius.is_a? Numeric or radius == 0
    super(name)
    @radius = radius
  end

  def area
    @radius ** 2 * Math::PI
  end

  def to_s
    "name: #{@name} - radius: #{@radius}"
  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

Why do you have a default radius = 0 if that is an unacceptable value? — 200_success
– 200_success, Commented Nov 29, 2017 at 0:20

Flambino · Accepted Answer · 2017-11-29 15:59:23Z

As mentioned in the comments by 200_success:

Why is zero the default, if it's invalid?
And: What about a negative radius?

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

Marc Rohloff · Accepted Answer · 2017-11-29 19:17:02Z

1

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

answered Nov 29, 2017 at 19:17

Marc Rohloff

3,54710 silver badges7 bronze badges

Add a comment |

Stack Exchange Network

Ensuring a non-zero radius when constructing a circle

2 Answers 2

You must log in to answer this question.

Hot Network Questions