Skip to content

Conversation

@jiegillet
Copy link
Contributor

Keeping the momentum :)

@jiegillet jiegillet added x:size/large Large amount of work x:type/coding Write code that is not student-facing content (e.g. test-runners, generators, but not exercises) x:module/practice-exercise Work on Practice Exercises labels Apr 15, 2022
@github-actions
Copy link
Contributor

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 🤖.

Copy link
Member

@angelikatyborska angelikatyborska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me. Fun short exercise! It can be merged as it is, or you can consider my proposal to decrease the difficulty first.

Comment on lines +1 to +26
defmodule KillerSudokuHelper do
@doc """
Return the possible combinations of `size` distinct numbers from 1-9 excluding `exclude` that sum up to `sum`.
"""
@spec combinations(cage :: %{exclude: [integer], size: integer, sum: integer}) :: [[integer]]
def combinations(%{exclude: exclude, size: size, sum: sum}) do
numbers = [9, 8, 7, 6, 5, 4, 3, 2, 1] -- exclude

do_combinations(numbers, size, [[]])
|> Enum.filter(&(Enum.sum(&1) == sum))
end

defp do_combinations(_numbers, 0, list), do: list

defp do_combinations(numbers, size, list) do
numbers
|> tails()
|> Enum.map(fn [head | tail] ->
do_combinations(tail, size - 1, Enum.map(list, &[head | &1]))
end)
|> Enum.concat()
end

defp tails([]), do: []
defp tails([_ | tail] = list), do: [list | tails(tail)]
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My solution, done without looking at yours first:

defmodule KillerSudokuHelper do
  @doc """
  Return the possible combinations of `size` distinct numbers from 1-9 excluding `exclude` that sum up to `sum`.
  """
  @spec combinations(cage :: %{exclude: [integer], size: integer, sum: integer}) :: [[integer]]
  def combinations(cage) do
    %{sum: sum, size: size, exclude: exclude} = cage
    sorted_numbers = Enum.to_list(1..9) -- exclude
    all_combinations = do_all_combinations(sorted_numbers, size)
    Enum.filter(all_combinations, &(Enum.sum(&1) == sum))
  end

  defp do_all_combinations(sorted_numbers, 1), do: Enum.map(sorted_numbers, & [&1])

  defp do_all_combinations(sorted_numbers, size) do
    Enum.flat_map(sorted_numbers, fn n ->
      sorted_numbers
      |> Enum.filter(&(&1 > n))
      |> do_all_combinations(size - 1)
      |> Enum.map(&[n | &1])
    end)
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very very similar, nice :)

config.json Outdated
"pattern-matching",
"recursion"
],
"practices": [],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, it could practice recursion or enum, but those are already full.

config.json Outdated
"recursion"
],
"practices": [],
"difficulty": 6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other difficulty 6 exercises are:
knapsack
list-ops
markdown
ocr-numbers
robot-simulator

I found this one much easier than those to be honest. Maybe a 4?

Suggested change
"difficulty": 6
"difficulty": 4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I agree.
Originally I placed it by knapsack because the problem is very very similar, however for this one, brute force is the best option since the size of the digits to chose is so constrained.

Comment on lines +78 to +82
@tag :pending
test "Cage with several combinations that is restricted" do
cage = %{exclude: [1, 4], size: 2, sum: 10}
assert KillerSudokuHelper.combinations(cage) == [[2, 8], [3, 7]]
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it much easier to start with this test instead of "Cage with sum 45 contains all digits 1:9" once I got the basic 1-element cases done because the exclude part was much easier to figure out correctly (-- exclude) than the recursion part 🤷 but I guess we will just stick to the order in the problem specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you usually solve these? Do you take it one test at a time the prescribed Exercism way?
I always do --include pending --seed 0 --listen-on-stdin and look at whichever failure interests me.
In that sense I never think too much about the order of the tests, but I don't mind switching tests around.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first run all tests on my initial attempt. If a lot of them fail, I go in order because most exercises have tests implemented in a good order for fulfilling them. I usually don't look at all the failures at once if it's more than 2.

As a maintainer, I prefer to keep the order of tests as it is in problem specs. That makes it easier to update the exercise later 😅

"""
@spec combinations(cage :: %{exclude: [integer], size: integer, sum: integer}) :: [[integer]]
def combinations(%{exclude: exclude, size: size, sum: sum}) do
numbers = [9, 8, 7, 6, 5, 4, 3, 2, 1] -- exclude
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you removed the descending range to make it pass in older Elixirs, but I would have still kept it, just with a Enum.sort instead. But it doesn't matter 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I first had 9..1//-1, and when it got rejected I wrote the full list explicitly out of spite :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:module/practice-exercise Work on Practice Exercises x:size/large Large amount of work x:type/coding Write code that is not student-facing content (e.g. test-runners, generators, but not exercises)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants