Warns about a compare function used in a sort

[chriseyre2000]

Here is the code:

defmodule School do
  @moduledoc """
  Simulate students in a school.

  Each student is in a grade.
  """
  @type school :: any()
  @doc """
  Create a new, empty school.
  """
  @spec new() :: school
  def new() do
    %{}
  end
  @doc """
  Add a student to a particular grade in school.
  """
  @spec add(school, String.t(), integer) :: {:ok | :error, school}
  def add(school, name, grade) do
    if school |> Map.get(name) != nil do
      {:error, school}
    else
      {:ok, school |> Map.put(name, grade)}
    end
  end
  @doc """
  Return the names of the students in a particular grade, sorted alphabetically.
  """
  @spec grade(school, integer) :: [String.t()]
  def grade(school, grade) do
    (for {k, v} <- school, v == grade, do: k) |> Enum.sort()
  end
  @doc """
  Return the names of all the students in the school sorted by grade and name.
  """
  @spec roster(school) :: [String.t()]
  def roster(school) do
    (for {k, v} <- school, do: {k, v} ) 
    |> Enum.sort(School)
    |> Enum.map(&elem(&1, 0))
  end
  def compare({name, grade}, {name2, grade2}) do
    cond do
        grade > grade2 -> :gt
        grade < grade2 -> :lt
        name > name2 -> :gt
        name < name2 -> :lt
        true -> :eq
      end
  end
end

It raises the following warning:

The public interface of a codebase should be carefully considered, as each public function increases long term maintenance costs and makes refactoring harder.

In every Exercism exercise, the public interface is defined by the test suite. If a function is being tested, it needs to the public. All other functions should be private, including functions with the same name but different arities.

# you defined the following public function

def compare(_, _)

# make it private like this

defp compare(_, _)

[jiegillet]

Just to be clear, your compare/2 is overriding the default behavior of comparison of a a 2-tuple? I had no idea you could do that. I also have no idea how to tackle this corner case...

[chriseyre2000]

It's one of the versions of Enum.sort, provide a module name with a compare function implemented in it. I am one of the crazy people who reads the documentation. It was intended to allow a struct to define how it is sorted, but here I am using it for a 2-tuple.

The lazy option would be to permit compare/2 to be defined if Enum.sort/2 is used.

[chriseyre2000]

I am also having trouble using this for the Poker exercise.

Here is my published solution that exhibits this issue: https://exercism.org/tracks/elixir/exercises/poker/solutions/chriseyre2000

[jiegillet]

Thank you for giving us more failing tests :)

Right now, I have 2 PRs lined up to fix #284 (in #287) and #276 (almost done but not pushed yet because it requires #287) that I would like to get done before tackling this issue. If you feel up to the task and you want to help, we could use a review on #287, it's a self-contained function, you don't need to know anything about the rest of the codebase.