Improved solution for smelodesousa/F5/5-brackets

[zazedd]

The solution for F5/5-brackets was over complicated and had some errors:

1. The accumulator collected all parenthesis in a list, and there was a check each time a new parenthesis was added to delete matching parenthesis. If the list was empty at the end, the phrase was valid. This can be done simply by keeping and integer accumulator and incrementing and decrementing at each left and right parenthesis respectively, and if the accumulator is 0 at the end the phrase is valid. This also frees up the implementation for the user, as the original input of the solution required an accumulator to be present, and this one does not.
2. The original implementation featured an incomplete match case and an unused variable.

[erikmd]

Thanks a lot @zazedd !

The new solution is indeed better! as it's more concise and without non-exhaustive pattern-matching.

However, it seems there's a new corner case: if text = [')'; '('] then the parentheses are unexpectedly accepted.

Do you think this code would be better?

let verify text =
  List.fold_left
    (fun acc c -> if acc < 0 then acc else match c with '(' -> acc + 1 | ')' -> acc - 1 | _ -> acc)
    0 text
  = 0

If yes, feel free to add another test in test.ml; and maybe, rephrase a bit more the descr.md so that the question does not impose a unique solution.

E.g. "… has well-placed parentheses. If you are not familiar with list iterators, feel free to write a local function involving an accumulator …"

WDYT?

[zazedd]

That totally went over my head!
I have updated the repository with the changes mentioned. The solution, the new test and the slight rephrasing of the description have been pushed.

[erikmd]

Thanks a lot @zazedd !

FYI the first CI job failed with this error:

smelodesousa/F5/5-brackets   [FAILED]
Exercise definition error while testing your solution:
File "/repository/exercises/smelodesousa/F5/5-brackets/test.ml", line 17, characters 12-25:
                                                       17 |   ~sampler: sample_verify
                                                                        ^^^^^^^^^^^^^
                                                       Error: This expression has type unit -> 'a list
but an expression was expected of type unit -> 'b * 'c
Type 'a list is not compatible with type 'b * 'c 


grader outcomes:
val print_html : 'a -> 'b = <fun>
module Code : sig val verify : char list -> bool end
module Solution : sig val verify : char list -> bool end
module Test_lib : Test_lib.S
module Report = Learnocaml_report
module Introspection = Introspection
val sample_verify_right : unit -> 'a list = <fun>
val sample_verify_wrong : unit -> 'a list = <fun>
val sample_verify : unit -> 'a list = <fun>

Maybe, could you try changing test_function_2_against_solution with test_function_1_against_solution ?

[zazedd]

Really sorry about the hold-up.
Changing the function seems to have worked. The tests that the ./deploy command runs, on the latest version of the learn-ocaml-public with the latest fix, now pass for me.
I am again really sorry for sending breaking code!

[erikmd]

No worries @zazedd !
The PR is fine now so I'm going to squash-merge.

BTW @AltGr I'm puzzled by the failure to build current learn-ocaml-corpus as is with learn-ocaml's master here:
https://github.com/ocaml-sf/learn-ocaml-corpus/actions/runs/7838323725/job/21403712820?pr=40

There is a bunch of Error: I/O error: _ albeit the CI command does not involve -v "$PWD:/repository":ro ↓

learn-ocaml-corpus/.github/workflows/test.yml

Line 30 in 5b94c8c

run: 'docker run --rm -v "$PWD:/repository" -v "$PWD/www:/home/learn-ocaml/www" ${{ matrix.server_image }} build'