In Guessing game analyzer reports no use of default arguments when they are stated

[konkasidiaris]

Hello awesome team! Elixir newbie here At guessing game the analyzer reports Essential Use a default argument in the compare function to handle a lack of guess.

From the documentation I get that the default values statement is correct (tests pass too!)

defmodule GuessingGame do
  def compare(secret_number, guess \\ '')
  def compare(_, guess) when not(is_number(guess)), do: "Make a guess"
  def compare(secret_number, guess) when secret_number == guess, do: "Correct"
  def compare(secret_number, guess) when abs(secret_number - guess) <= 1, do: "So close" 
  def compare(secret_number, guess) when secret_number < guess, do: "Too high" 
  def compare(secret_number, guess) when secret_number > guess, do: "Too low"
end

Is there anything wrong with the aforementioned code or it is a false report of elixir analyzer?

Thank you for your time!

[angelikatyborska]

Hi!

Is there anything wrong with the aforementioned code or it is a false report of elixir analyzer?

I'd say there's a little bit of an error on both sides 😉 The analyzer expects a specific value of the default argument - the atom :no_guess, as is suggested by the code example in the exercise instructions.

Maybe we should modify the test suite to ensure that non number guesses other than that specific atom raise an error?

[konkasidiaris]

I am not sure how you want to run this!

1. Is there a pattern in elixir that suggests every default value should be an atom? I find this a bit peculiar to be honest.
2. Also the error message is a bit misleading as it suggest to use a default argument when there is already a default argument.
3. The :no_guess atom is suggested in the example but what is the point of trying to solve the exercise if we just c/p the hints and examples?

These are a couple of questions to help you get into the shoes of an elixir newbie again!

[jiegillet]

Hi @konkasidiaris, is it me or do you sound a bit confrontational? No one is suggesting that every default value should be an atom, and Angelika admitted to having errors on both sides. Learning exercises are made to nudge students towards a specific solution, deemed idiomatic. In this case we were nudging towards using :no_guess (which is certainly more idiomatic than an empty charlist).

We have some courses of action here:

1. modify the analyzer to fix the misleading error message
2. add a hint and some tests to clarify that we expect :no_guess specifically

Would you like to help with any of that?

[konkasidiaris]

Hello @jiegillet, I did not tried to sound like this by any means! If this is the case I am really sorry I was just expressing the questions that are in my mind right now, nothing more, no shots fired to anyone

Both actions sound good first issues, may I try to implement them?

[jiegillet]

Hello @jiegillet, I did not tried to sound like this by any means! If this is the case I am really sorry

It's ok, that impression was based on turns of phrase like "I find this a bit peculiar to be honest" and "what is the point of trying to solve the exercise" that can be interpreted as being condescending. Anyway, I'm glad you didn't mean it like that and I'm glad you want to help.

For the analyzer one, this needs to be modified:

https://github.com/exercism/elixir-analyzer/blob/4ba9a6ac960a84499c53fd3d99b9db72ddc0d09a/lib/elixir_analyzer/test_suite/guessing_game.ex#L21-L41

Replacing :no_guess by _ignore should do the trick to detect any kind of default argument. We also need to add tests somewhere here

https://github.com/exercism/elixir-analyzer/blob/4ba9a6ac960a84499c53fd3d99b9db72ddc0d09a/test/elixir_analyzer/test_suite/guessing_game_test.exs#L34-L38

We need to add tests for another solution that doesn't use :no_guess, yours would be great for that actually.

Sounds good? Let me know if you have more questions or if you change your mind about implementing the change.

[jiegillet]

I didn't hear from you, so I made a fix in #345