exercism / elixir-analyzer

GNU Affero General Public License v3.0
30 stars 31 forks source link

Remote Control Car: use default argument when defining function `new` #403

Closed sanderploegsma closed 9 months ago

sanderploegsma commented 9 months ago

The analyzer suggested to use a default argument when defining the function RemoteControlCar.new() on the solution below. I believe that is what I'm doing, so I wonder whether this is a bug in the analyzer.

defmodule RemoteControlCar do
  @enforce_keys [:battery_percentage, :distance_driven_in_meters, :nickname]
  defstruct [:battery_percentage, :distance_driven_in_meters, :nickname]

  @default_nickname "none"

  def new(nickname \\ @default_nickname) do
    %RemoteControlCar{
      battery_percentage: 100,
      distance_driven_in_meters: 0,
      nickname: nickname
    }
  end

  def display_distance(%RemoteControlCar{distance_driven_in_meters: d}), do: "#{d} meters"

  def display_battery(%RemoteControlCar{battery_percentage: 0}), do: "Battery empty"
  def display_battery(%RemoteControlCar{battery_percentage: p}), do: "Battery at #{p}%"

  def drive(%RemoteControlCar{battery_percentage: 0} = remote_car), do: remote_car

  def drive(%RemoteControlCar{battery_percentage: p, distance_driven_in_meters: d} = remote_car) do
    %{remote_car | battery_percentage: p - 1, distance_driven_in_meters: d + 20}
  end
end
jiegillet commented 9 months ago

Hi @sanderploegsma, thank you for reporting this. Yes, you are correct, this is a bug. The analyzer in this file does not expect a module attribute as a default argument, only the string "none".

I believe the fix would be to replace all instances of "none" in that file and replace it by _ignore. I don't think we will miss anything, because the tests check the value of the argument.

Would you be willing to make that change? It would also require new tests (like your solution) for test_exercise_analysis "using default arguments for new" in this file.

sanderploegsma commented 9 months ago

Thanks for the detailed description! Yes I’d like to open a PR to fix it. 😁