Analysis of Wine Cellar

[jayber]

Hello

I love your work. I am only just learning Elixir but wonder if the feedback given as Essential in this exercise might be incorrect in these circumstances. Keyword lists can contain more than one value for a key, which makes retrieving all values of a key cumbersome. Keyword module, besides Keyword.get to get a single value, also provides Keyword_get_values to get multiple values. Use Keyword.get_values to filter wines by color.

However the solution I provided didn't use either get or get_values, but just iterated over the keyword list in the same style as the other functions provided as part of the exercise.

  def explain_colors do
    [
      white: "Fermented without skin contact.",
      red: "Fermented with skin contact using dark-colored grapes.",
      rose: "Fermented with some skin contact, but not enough to qualify as a red wine."
    ]
  end

  def filter(cellar, color, opts \\ []) do
    filter_by_color(cellar, color) |> further_filter(opts)
  end

  defp further_filter(cellar, []), do: cellar

  defp further_filter(cellar, [{:year, year} = _head | tail]) do
    filter_by_year(cellar, year) |> further_filter(tail)
  end

  defp further_filter(cellar, [{:country, country} = _head | tail]) do
    filter_by_country(cellar, country) |> further_filter(tail)
  end

  defp filter_by_color([], _color), do: []

  defp filter_by_color([{color, wine} = _head | tail], color) do
    [wine | filter_by_color(tail, color)]
  end

  defp filter_by_color([_head | tail], color) do
    filter_by_color(tail, color)
  end

  # The functions below do not need to be modified.

  defp filter_by_year(wines, year)
  defp filter_by_year([], _year), do: []

  defp filter_by_year([{_, year, _} = wine | tail], year) do
    [wine | filter_by_year(tail, year)]
  end

  defp filter_by_year([{_, _, _} | tail], year) do
    filter_by_year(tail, year)
  end

  defp filter_by_country(wines, country)
  defp filter_by_country([], _country), do: []

  defp filter_by_country([{_, _, country} = wine | tail], country) do
    [wine | filter_by_country(tail, country)]
  end

  defp filter_by_country([{_, _, _} | tail], country) do
    filter_by_country(tail, country)
  end
end

Is there a difference that means this style is ok in one case and not in another, or is it more like the point of the exercise is to use Keyword module? Also if someone was incorrectly using get instead of get_values it would be caught by the tests, wouldn't it? This advice is in Hints anyway, so is it necessary?

Hope this is useful.

[jayber]

Ok, I've looked at other solutions and see why the Keywords module solutions are better (more compact). I guess I was led astray by the examples of the supplied filters. I can see you didn't want to provide the answer in them, so you did it manually. Hmm maybe you can just remove those implementations, so they have to be provided by the candidate, using the hoped for Keyword module. They are just the same thing, aren't they?

[angelikatyborska]

Hi! Thank you for the issue!

They are just the same thing, aren't they?

No, not at all. The functions provided in the boilerplate, filter_by_year and filter_by_country, don't use the concept of the exercise - keyword lists. Their first argument, called wines, is just a regular list. It contains 3-element tuples with strings and integers, for example [ {"Chardonnay", 2015, "Italy"}]. For a list to be a keyword list, it needs to contain two-element tuples where the first element is an atom.

That's why those functions are provided in the boilerplate. So that you can focus on learning keyword lists without having to implement something unrelated to keyword lists.

The intention of the comment was to make you use Keyword.get_values. It seems like the comment failed at that because you needed to look at other people solution's to get convinced 🙂. Do you think we need to rephrase the comment?

[jayber]

Wow, so quick to respond. Thank you! Aah I see. I suppose it suggested to me that not using a get_values solution would fail, rather than it was just avoiding the point of the exercise. Maybe it should say something like that for people like me who have managed to miss it. "The point of the exercise is to learn about keyword lists, not lists in general. If you haven't used the functions available in the Keyword module, you might have missed the point." Since just treating them like lists still works fine... Although I can see why you'd think that a readme full of direction would not be missed... ;-) Anyway I hope I haven't wasted your time. Exercism really is an incredible achievement and one I'd like to make a meaningful contribution to one day. Thank you.

[jayber]

I mean: I got it wrong. If you want to cater for slow coaches like me maybe changing the message would help. :-) But maybe it's not necessary

[jiegillet]

Even if it's not perfect, the comment explicitly mentions what functions to use, so I think the message still works for this case. I will close the issue, but we will keep an eye on future similar issue for this exercise.