exercism / elixir-analyzer

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

Square Root. Analyzer seems to complain about the use of any Kernel function and not just sqrt(). #405

Closed heptalophos closed 11 months ago

heptalophos commented 11 months ago

I have got the following complaint from the Analyzer:

Essential The purpose of this exercise is to construct an algorithm to compute an integer square root. Do not use built-in square root functions such as :math.sqrt, :math.pow or Float.pow.

after submitting this iteration:

defmodule SquareRoot do
  @doc """
  Calculate the integer square root of a positive integer,
  using the exponential identity:
              sqroot(x) = e ** (1/2 * ln(x))
  """
  @spec calculate(radicand :: pos_integer) :: pos_integer
  def calculate(radicand) do
    # Hardcode an approximation of Euler's number, as :math.e is private
    e = 2.718281828459045
    # Calculate a reasonable approximation for ln(x)
    nat_log = fn x -> 99999999 * (x ** (1 / 99999999) - 1) end
    # Take the calculated root-approximation's integer part
    floor (e ** (0.5 * nat_log.(radicand)))
  end
end

As you can see, I have gone out of my way to avoid using a reference to any :math functions, to the point of hard-coding Euler's number e - which is private anyway in :math, as it's mentioned in my comment above (at first I thought that this might have triggered the complaint 😀) - and defining an anonymous function to approximate ln(x), so as to not use :math.ln/1.

Then I used Kernel.floor/1 because the nat_log approximation slightly overshoots the integer square root. Initially I thought that Analyzer didn't like it, because floor/1 returns a float and the @spec demands a pos_integer so I changed the last line to trunc (e ** (0.5 * nat_log.(radicand))) in the next iteration.

And I got exactly the same response.

And then I checked The Analyzer's test-suite and found this curious assertion:

assert_no_call "do not use **" do
    type :essential
    called_fn module: Kernel, name: :**
    comment Constants.square_root_do_not_use_built_in_sqrt()
  end

🙂

jiegillet commented 11 months ago

Hi @heptalophos! It works as intended from my point of view, ** is the power operator and the point of this exercise is to implement yourself an algorithm for computing the square root without built-in methods. Otherwise you could just do floor (radicand ** 0.5) and be done with it. Your solution is fancier, but is essentially the same.

May I show you this link? Plenty of fun algorithms to try out :)

angelikatyborska commented 11 months ago

@jiegillet Yup, but I think we forgot to mention Kernel.** in the comment, I'm adding it here: https://github.com/exercism/website-copy/pull/2282

heptalophos commented 11 months ago

Thanx.

I still think though that completely blanketing out an essential arithmetical operator like (needed -for instance- at a previous point for a quite different calculation), is a bit sloppy. And no, the exponential identity is not actually essentially the same as doing `floor (radicand 0.5)`.

It is in fact included in "the plenty of fun algorithms to try out" in that link 😀