elixir-analyzer
elixir-analyzer copied to clipboard
Accept defdelegates as valid equivalents of function calls in `assert_call`
Hi, I'd like to start off with saying I love this tool and the exercises.
Now I've been using Elixir for a couple of years already and I'm kinda trying to work my way through the problems as fast as possible, and I implemented the "High Score' problem using defdelegate
. Now I understand this isn't the point of the exercise, but the analysis remarks I got weren't correct.
This is the code:
defmodule HighScore do
@default_score 0
defdelegate new, to: Map
defdelegate add_player(scores, name, score \\ @default_score), to: Map, as: :put
defdelegate remove_player(scores, name), to: Map, as: :delete
defdelegate reset_score(scores, name), to: __MODULE__, as: :add_player
def update_score(scores, name, score), do: Map.update(scores, name, score, fn x -> score + x end)
defdelegate get_players(scores), to: Map, as: :keys
end
This was the feedback of the analysis:
Essential Use a module attribute in add_player/3 and reset_score/2 as a constant for the initial score. Recommended Use a default argument in the add_player/3 function to handle a lack of initial score. Use the module attribute with the default initial score.
Even though the defdelegate
s are probably cheating, I am using default arguments and a module attribute, so IMO the analysis is not correct.
Maybe it's an option to instead add feedback to not use defdelegate
as well, as it defeats the purpose of the exercise.
TIL about defdelegate
:)
I wouldn't expect beginners to know about it either, but that doesn't mean it's cheating to use it.
That being said, it might be a little tricky to fix the essential comment, so I think I would go with your last suggestion.
Hi!
First of all, I'm really sorry it took me so long to get back to you 🙏. It has been a very busy summer for me.
Second of all, thank you for reporting this and bringing this issue to our attention.
I'm not entirely sure if I agree with this statement:
not use defdelegate as well, as it defeats the purpose of the exercise.
I think a solution with defdelegate
s like yours is perfectly valid. You still had to choose all the right Map
module functions and know how to use them. The fact that you used them via defdelegate
instead of good old function calls doesn't make a difference here in my opinion. For this reason, I would be against telling students not to use it.
However, as @jiegillet pointed out, fixing the analyzer to accept defdelegate
s as valid equivalents of function calls might be a bit tricky, but I still think that's the right course of action. I'll attempt it when I have a bit more time.
For now, a quick fix could be to ignore all assert_calls
for this exercise if there's any defdelegate
present. There's a suppress_if
option in our custom macros that should make this possible.
This is still not working correctly. I submitted a solution almost identical to this one, and the "default argument" feedback still appears.
@bugQ Can you please copy-paste your solution so we can check?
@angelikatyborska
defmodule HighScore do
@default_score 0
def new(), do: %{}
defdelegate add_player(scores, name, score \\ @default_score), to: Map, as: :put
defdelegate remove_player(scores, name), to: Map, as: :delete
def reset_score(scores, name), do: Map.put(scores, name, @default_score)
def update_score(scores, name, score), do: Map.update(scores, name, score, fn x -> x + score end)
defdelegate get_players(scores), to: Map, as: :keys
end
Feedback:
Recommended Use a default argument in the add_player/3 function to handle a lack of initial score. Use the module attribute with the default initial score.
@bugQ Thanks, this should fix it: https://github.com/exercism/elixir-analyzer/pull/372