exercism / elixir-analyzer

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

Universal recommendations? #310

Closed Rutaba closed 2 years ago

Rutaba commented 2 years ago

So I just completed the Lasagna Learning Exercise in the Elixir Track and after submitting the assignment, the recommendation of re-using the functions was still issued when I already had re-used them in my code.

This is the code that I submitted:

` defmodule Lasagna do def expected_minutes_in_oven, do: 40

def remaining_minutes_in_oven(time_in_oven) do expected_minutes_in_oven - time_in_oven end

def preparation_time_in_minutes(layers), do: layers * 2

def total_time_in_minutes(layers, time_in_oven) do preparation_time_in_minutes(layers) + time_in_oven end

def alarm, do: "Ding!" end ` Opening the issue here in-case this is a real issue, otherwise please let me know if I am missing something.

Rutaba commented 2 years ago

I had the comments that are initially there on the start of the exercise. When I re-submitted after removing those, I did not got the re-using functions recommendation again

angelikatyborska commented 2 years ago

The problem is this:

def remaining_minutes_in_oven(time_in_oven) do
  expected_minutes_in_oven - time_in_oven
end

expected_minutes_in_oven is not recognized as a function call because it's missing (). We need to fix that in the analyzer. Thank you for reporting!

angelikatyborska commented 2 years ago

@Rutaba I looked at the analyzer code closely and I noticed that we actually chose not to handle this case explicitly. You should have also received a compilation warning that would tell you to add the missing ():

Screenshot 2022-07-30 at 19 17 29
warning: variable "expected_minutes_in_oven" does not exist and is being expanded to "expected_minutes_in_oven()", please use parentheses to remove the ambiguity or change the variable name
  lib/lasagna.ex:8

If you did get the compilation warning, I'm going to consider the analyzer working as expected and close this. If you didn't get a compilation warning, let me know.