soegaard / sketching

A Racket library for creative drawings and animations. Inspired by Processing.
110 stars 10 forks source link

Misleading error message when calling size with numbers less than 100 #50

Closed Ecsodikas closed 2 years ago

Ecsodikas commented 2 years ago

Howdy, I think I found an misleading error message:

when calling the (size w h) function in setup with w or h less than 100 the error I get is:

; min-height in area<%>: contract violation                          <--- or min-width, depends on w or h being less than 100
;   expected: (integer-in 0 1000000)
;   given: #f

The irritating part is that the error tells me that a number between 0 1000000 is possible, but #f is given. So this area<%> error message is propagating instead of a error telling us that the lowest resolution is 100x100.

Ecsodikas commented 2 years ago

Because I have never done anything with error handling in Racket I'd like to give it a shot in fixing it.

If you even want this to be changed.

soegaard commented 2 years ago

Please do!

Ecsodikas commented 2 years ago

I fixed the problem, but I probably found another bug by doing so.

The guard-function make-at-least-guard is defined in the following way:

(define (make-at-least-guard n)
  ; BUG?: Should this return #f or at least n?
  (λ (x) (and (positive-integer? n) (>= x n) x)))

Reading the name, I suspect the function to return either the given value of x or n. But it returns #f if x is less than n, which breaks the area<%> contract.

So, should I check for a value less than 100 before the area<%> gets created and raise an error, or should I change the guard to actually set the input to the minimum? I do not know if this would break something, so I just added some conditional checks to check for a value less than 100.

soegaard commented 2 years ago

Let's see. Explaining to myself here.

A parameter can have a "guard". When the user sets the parameter to a new value, the guard is called and the parameter is set to what the guard returns. In the normal case it should return the new value x.

If the guard discovers that the new value is not appropriate for the parameter, it must reject the new values. And ... yep ... there is a bug.

When a value is rejected, the guard should either:

It's not enough to return #f which just sets the parameter to #f. [I don't know where I got that idea.]

The question is what the best choice is. Since size is a call-once function I think it makes sense to throw an exception with a message that explains that the minimum widht/height must be a 100.

Without any testing at all, the guard for the width ought to look like this:

EDIT: Since the minimum width is 100, there no need for n.

(define (width-guard x)
     (unless (and (positive-integer? x) (>= x 100))
         (raise-arguments-error 'width
            "width needs to be at least 100"
            "width" x)))
      x)

Similarly a new guard is needed for the height.

If the guards works as intended, then size can't set width and height to values smaller than 100. And therefore the problem you saw area<%> can't happen.

Do you want to fix the guards?

Ecsodikas commented 2 years ago

Thanks for the insight. I'm happy to fix those tonight.