[#792] New practice exercise `two-bucket` #808

jiegillet · 2021-07-02T14:13:02Z

One more.

github-actions · 2021-07-02T14:13:16Z

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:

General steps
- 🏆 Does this PR need to receive a label with a reputation modifier (x:size/{tiny,small,medium,large,massive})? (A medium reputation amount is awarded by default, see docs)
Any exercise changed
- 👤 Does the author of the PR need to be added as an author or contributor in <exercise>/.meta/config.json (see docs)?
- 🔬 Do the analyzer and the analyzer comments exist for this exercise? Do they need to be changed?
- 📜 Does the design file (<exercise>/.meta/design.md) need to be updated to document new implementation decisions?
Practice exercise changed
- 🌲 Do prerequisites, practices, and difficulty in config.json need to be updated?
- 🧑‍🏫 Are the changes in accordance with the community-wide problem specifiations?
Practice exercise tests changed
- ⚪️ Are all tests except the first one skipped?
- 📜 Does <exercise>/.meta/tests.toml need updating?

Automated comment created by PR Commenter 🤖.

angelikatyborska · 2021-07-02T15:00:05Z

config.json

+          "pattern-matching",
+          "if",
+          "cond",
+          "match",


This isn't a concept 🤔 what did you mean by "match"?

I think I mean case, I was pretty tired when I wrote that -_-

angelikatyborska · 2021-07-02T15:10:24Z

config.json

+        "practices": [
+          "atoms",
+          "integers"
+        ],


Those are some pretty basic concepts for a rather complex exercise 🤔 and the check is failing for integers... maybe we could leave it completely empty? Or we say this is specifically a tail call recursion exercise and it practices and requires tail-call-recursion?

This is not really a tail call recursion, it's more of a breadth first search algorithm. I don't remember why I wrote these two. I think it's best to leave empty.

Oh I remember now, copy paste :(
Sorry about that!

angelikatyborska · 2021-07-02T15:13:16Z

exercises/practice/two-bucket/.meta/example.ex

+      # Pour one into the other
+      {min(size_one, fill_one + fill_two), max(0, fill_two - size_one + fill_one)},
+      {max(0, fill_one - size_two + fill_two), min(size_two, fill_one + fill_two)}


Normally I don't comment on the example solution, but I'm actually struggling to understand this exercise. Could you explain to me why min and max are necessary? I would think that when you pour from one bucket into the other until the first one is full, there is only one possible outcome.

No worries, I'm happy to explain. There is only one possible outcome indeed, but you have to be careful how to calculate it and I'm using min/max as a shortcut not to use an if statement. For example, imagine we want to pour the first bucket in the other one (line 51). There are 2 cases:

There is enough room in the second bucket for all the water => result is 0 in the first bucket

There is not enough room in the second bucket => result is current fill fill_one minus the remaining room in the second bucket size_two - fill two.
In general, I can calculate fill_one - (size_two - fill_two) and if it's negative, it means that there was extra room in the second bucket but since columns can't be negative, I need to cap that to 0. Hence max(0, fill_one - (size_two - fill_two)). If the same as if fill_one < size_two - fill_two, do: 0, else: fill_one - (size_two - fill_two) .

It's a similar process for the other bucket, but I use min to prevent over-fullness.
Does that make sense? Should I use an if statement instead?

Makes sense now, thanks! No, there's no need to change it to an if :)

angelikatyborska · 2021-07-02T15:14:22Z

exercises/practice/two-bucket/.meta/example.ex

+      |> Enum.reject(&MapSet.member?(forbidden_states, &1))
+      |> Enum.map(&{&1, moves + 1})
+
+    pour(states ++ next, MapSet.put(forbidden_states, state), next_moves, goal)


This is just a generic worry, I haven't analyzed the solution in detail, so it might be misguided, but:

Appending to a list in a loop? Are you sure this is the best way?

Yes, because this is a breadth first search. I need to look at states with a lower number of moves first, and the next states I just created last. Otherwise I would need to generate all paths and check for the lowest after.
A queue is the best data structure for this, but it's not worth implementing in this solution I think (and also the standard FP queue is made out of two lists and use that infamous Enum.reverse that you don't like :p ).

All good. My comment about two lists and Enum.reverse was purely in the context of double linked lists 👻.

If you need a queue, use Erlang's (http://erlang.org/doc/man/queue.html). It has functions for adding and removing from both ends. And yes, it's two lists 😁

neenjaw · 2021-07-02T16:58:30Z

exercises/practice/two-bucket/.meta/example.ex

@@ -0,0 +1,55 @@
+defmodule TwoBucket do
+  defstruct [:goal, :moves, :other_bucket]
+  @type t :: %TwoBucket{goal: :one | :two, moves: integer, other_bucket: integer}


Is this the clearest interface?

Would it not be clearer for a student studying the problem to return their answer using a struct %TwoBucket{bucket_one: x, bucket_two: y, moves: z}

It's the interface suggested in the tests. But you are right it's not super intuitive, I'll change it. Thank you!

Cool, I think things like this are good reasons to break continuity with the problem specs repo

Agreed! It reads much better now

angelikatyborska

Did I mention how happy I am that you're adding all of those complex exercises so I don't have to solve them myself? 🥳

jiegillet · 2021-07-03T23:13:37Z

No worries, I'm a problem solver, in this case quite literally :)

jiegillet added 3 commits June 30, 2021 18:39

New practice exercise

f625dfe

Merge branch 'main' of github.com:exercism/elixir into main

7fefb66

[exercism#792] New practice exercise:

4245289

angelikatyborska reviewed Jul 2, 2021

View reviewed changes

angelikatyborska added the x:size/large Large amount of work label Jul 2, 2021

neenjaw reviewed Jul 2, 2021

View reviewed changes

jiegillet added 3 commits July 3, 2021 16:52

Update prerequisites

c2ab88d

Contributors

9268b04

Change output type

33d781f

angelikatyborska approved these changes Jul 3, 2021

View reviewed changes

neenjaw approved these changes Jul 3, 2021

View reviewed changes

angelikatyborska merged commit 0c41cbd into exercism:main Jul 3, 2021

jiegillet deleted the jie-two-bucket branch July 3, 2021 23:13

angelikatyborska mentioned this pull request Jul 4, 2021

New practice exercise two-bucket #792

Closed

This was referenced Jul 14, 2022

Document reputation label exercism/docs#347

Merged

Consider correct reputation amounts for different labels exercism/exercism#6440

Open

Consider correct reputation amounts for different labels exercism/exercism#6441

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#792] New practice exercise `two-bucket` #808

[#792] New practice exercise `two-bucket` #808

jiegillet commented Jul 2, 2021

github-actions bot commented Jul 2, 2021

angelikatyborska Jul 2, 2021

jiegillet Jul 2, 2021

angelikatyborska Jul 2, 2021

jiegillet Jul 2, 2021

jiegillet Jul 3, 2021

angelikatyborska Jul 2, 2021

jiegillet Jul 3, 2021 •

edited

Loading

angelikatyborska Jul 3, 2021

angelikatyborska Jul 2, 2021

jiegillet Jul 3, 2021

angelikatyborska Jul 3, 2021

neenjaw Jul 2, 2021 •

edited

Loading

jiegillet Jul 3, 2021

neenjaw Jul 3, 2021

angelikatyborska Jul 3, 2021

angelikatyborska left a comment

jiegillet commented Jul 3, 2021

[#792] New practice exercise two-bucket #808

[#792] New practice exercise two-bucket #808

Conversation

jiegillet commented Jul 2, 2021

github-actions bot commented Jul 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiegillet Jul 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neenjaw Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angelikatyborska left a comment

Choose a reason for hiding this comment

jiegillet commented Jul 3, 2021

[#792] New practice exercise `two-bucket` #808

[#792] New practice exercise `two-bucket` #808

jiegillet Jul 3, 2021 •

edited

Loading

neenjaw Jul 2, 2021 •

edited

Loading