How to handle input validation and other exceptions in exercises? #670

michalporeba · 2024-09-13T10:35:52Z

Context

Some exercises require input validation. A simple example (copied below) from the grains exercise.

{
      "uuid": "61974483-eeb2-465e-be54-ca5dde366453",
      "description": "negative square is invalid",
      "property": "square",
      "input": { "square": -1 },
      "expected": { "error": "square must be between 1 and 64" }
}

At the moment, the Clojure track lacks a consistent approach to what to expect from learners and, therefore, how to implement the tests.

A consistent approach is important to allow learners to focus on the core of the exercise, on the concept which is being presented without needing to learn various ways of error handling that are possible in Clojure. Error handling should be explored with specific examples in an error-handling concept.

For the same reason, we should choose a simple or the simplest approach.

But it should also be idiomatic so that we are not introducing practices which are not natural for the language.

Our options

Option 1 - return error value

Some of the current exercises return an "error value", for example in the phone number exercise have:

(deftest invalid-with-letters
  (testing "Invalid with letters"
    (is (= "0000000000" (phone/number "523-abc-7890")))))

This could be standardised to include the expected description of the error:

(deftest invalid-with-letters
  (testing "Invalid with letters"
    (is (= "error: phone number is invalid with letters" (phone/number "523-abc-7890")))))

Pros: The implementation of the phone/number could be left to the learner. Any flow control would do.
Cons: It is generally not a good practice as error handling might be difficult.

Option 2 - asserts

A relatively common pattern in Clojure is to use asserts. An example of testing could be the following:

(deftest test-zero-guard
  (testing "error handling test"
    (is (thrown? AssertionError (divide 2 0)))))

or with the exact message

(deftest test-zero-guard
  (testing "error handling test"
    (is (thrown-with-msg?
         AssertionError
         #"Cannot divide by 0!"
         (divide 2 0)))))

with the expected implementation implementation something like this:

(defn divide [a b]
  (assert (not (zero? b)) "Cannot divide by 0!")
    (/ a b))

Pros: Very simple and relatively common approach. Multiple assertions can be added one after the other without increasing nesting.
Cons: Strong voices in the community suggest that assertions use internal Java constructs that were never meant for such use, only for fatal errors that should result in the application stopping immediately.

Option 3 - throwing exceptions

Instead of using assertions we can throw IllegalArgumentException or a more generic exception with ex-info. The testing would be the same as in Option 2 - asserts, but the implementation would change to:

(defn divide [a b]
  (if (zero? b) (throw (IllegalArgumentException. "Cannot divide by 0!"))
  (if (= 3 b) (throw (IllegalArgumentException. "We don't like threes"))
  (/ a b))))

or with ex-info

(defn divide [a b]
  (if (zero? b) (throw (ex-info "Cannot divide by 0!" {}))
  (if (= 3 b) (throw (ex-info "We don't like threes" {}))
  (/ a b))))

Pros: Perhaps a more standard approach to error handling, but it requires conditionals, constructing, and throwing exceptions.
Cons: More complex, and with multiple assertions it adds a lot to the actual code for the solution.

Option 4 - semantic error handling

This is similar to error codes, Option 1, but with a little bit more structure. Testing could look like so:

(deftest test-zero-guard
  (testing "error handling test"
    (is (= {:error "Cannot divide by 0!"}
         (divide 2 0)))))

(deftest test-for-three-handling
  (testing "we don't like threes"
    (is (contains? (divide 2 3) :error))))

With an example implementation code implementing it here:

(defn divide [a b]
  (if (zero? b) {:error "Cannot divide by 0!"}
  (if (= 3 b) {:error "We don't like threes"}
  {:result (/ a b)})))

Pros: An approach that might be familiar from other languages. It provides clear structure to the tests.
Cons: This can be a bit verbose for simple exercises requiring {:result (whatever)} for the return.

The text was updated successfully, but these errors were encountered:

michalporeba · 2024-09-13T10:37:13Z

@tasxatzial My favourite option at the moment is to use asserts for its simplicity and then cover the other options in a concept dedicated to error handling to explain how to do it in the other way described above. What is your opinion, your preference?

michalporeba · 2024-09-13T10:41:06Z

I have not included the option of using specs because they are even more complex, and I think this should be introduced in a dedicated concept.

tasxatzial · 2024-09-13T16:21:09Z

I agree that we should be choosing the simplest approach.

However, since exceptions aren't really the main focus of the exercises, we should carefully consider whether breaking all solutions simply because we changed the returned error is worth the trouble. Once the solutions are invalidated, people are unlikely to update them, especially since Clojure isn't very popular. Consequently, new learners will end up looking at broken solutions without any indication of why they are broken. This doesn’t sound like a small problem.

Additionally, we should keep in mind that syncing to the specs will already invalidate all solutions for specific exercises for reasons unrelated to error handling.

Idiomatic return value: I would only consider returning nil instead of an error if it's more appropriate for a specific exercise.

Assertions: Whether they simplify the code or not should never be a decisive factor, even if Clojure code already appears messy. One more level of nesting doesn't make a significant difference. If it does, that's an opportunity for refactoring. Additionally, none of the 80 exercises I have locally use assertions; all of them use exceptions. Therefore, switching to assertions would immediately break all solutions without providing any real added value.

Skipping tests: I'm always in favor of skipping an error test case instead of adding it and breaking all solutions. For example, the grains exercise suggests returning errors for invalid input. Does including those tests add any significant value? Not really—it's just extra tests that cover more cases. It won’t cause any harm if they are not included; after all, the problem specs are only suggestions. We are free to skip any tests and keep the exercise simple.

My preference: Return a simple Exception object in the most generic case, or a more specific object like IllegalArgumentException. Whether the tests also need to check the error message is something that needs to be carefully considered. More often than not, it's sufficient to check only if the user has accounted for the special case and thrown an exception. If an exercise has many error cases, then maybe the semantic error handling would be more appropriate.

Bottom line: I would prefer that old exercises are invalidated only for very good reasons, such as tests being completely outdated or inconsistent. For new exercises, options like exceptions, nil, semantic errors, and skipping tests are all on the table.

tasxatzial · 2024-09-14T18:09:02Z

After thinking about it for a while, i'm realizing that solving this problem is vanity. Take a look at how different Clojure functions behave:

(get [1 2] -1) -> nil
(nth [1 2] -1) -> IndexOutOfBoundsException
(flatten -1) -> ()

In the first example, we are trying to access an element at index -1, and the get function simply returns nil. This value doesn't even indicate whether the index exists, as nil could easily be a valid element in the vector.

In the second example, nth throws an exception instead.

In the third example, we pass a number to flatten, which obviously won't work since the function operates on sequences. Yet, it returns an empty list.

At this point, it looks like we should stick with what we already have. This does not seem like a problem we'll be able to solve in a satisfactory way.

tasxatzial · 2024-11-08T13:08:40Z

Current list of implemented exercises that include error handling

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

How to handle input validation and other exceptions in exercises? #670

How to handle input validation and other exceptions in exercises? #670

michalporeba commented Sep 13, 2024

michalporeba commented Sep 13, 2024

michalporeba commented Sep 13, 2024

tasxatzial commented Sep 13, 2024 •

edited

Loading

tasxatzial commented Sep 14, 2024 •

edited

Loading

tasxatzial commented Nov 8, 2024 •

edited

Loading

How to handle input validation and other exceptions in exercises? #670

How to handle input validation and other exceptions in exercises? #670

Comments

michalporeba commented Sep 13, 2024

Context

Our options

Option 1 - return error value

Option 2 - asserts

Option 3 - throwing exceptions

Option 4 - semantic error handling

michalporeba commented Sep 13, 2024

michalporeba commented Sep 13, 2024

tasxatzial commented Sep 13, 2024 • edited Loading

tasxatzial commented Sep 14, 2024 • edited Loading

tasxatzial commented Nov 8, 2024 • edited Loading

tasxatzial commented Sep 13, 2024 •

edited

Loading

tasxatzial commented Sep 14, 2024 •

edited

Loading

tasxatzial commented Nov 8, 2024 •

edited

Loading