Non Functional Code refactor

Questions / Help
1

hi guys, I was asked to refactor this function:

  def valid_signer_account?(private_key) do
    with {:ok, pair} <- Helper.get_decoded_keypair_from_secret(private_key),
         {:ok, account} <- Accounts.get_account_by_public_key(pair.public),
         "signer" <- account.type do
      {:ok, true}
    else
      _ -> {:ok, false}
    end
  end

I most say that I’m not elixir expert, but I can’t find what more can i do there since the cases were refactored using the “with” and i was trying to split into some functions clauses but guards doesn’t accept complex conditions.
I was wondering if there is something else I can do here to optimize the function.

Thanks in advice!!

2

Why do you have an ok tuple when all of your possible results are ok? There’s a certain convention about functions that end in a ?

3

yeah, you are right, but I didn’t code that part, at this moment I was noticed to take some specific functions and refactor them, since some of that functions are not full functional code, but in this case I can’t find a way to split into function clauses or use guards

4

In this case you could make the pattern match more explicit:

  def valid_signer_account?(private_key) do
    with {:ok, %{public: public}} <- Helper.get_decoded_keypair_from_secret(private_key),
         {:ok, %{type: "signer"} <- Accounts.get_account_by_public_key(public) do
      {:ok, true}
    else
      _ -> {:ok, false}
    end
  end

Not sure though if it gains you much.

To be honest, without knowing more about the domain, I don’t see much refactoring potential here.

2 Likes
5

Aside from the output strangeness I’d say the code looks reasonable.

1 Like
6

yeah I thought the same, I was thinking if splitting the with cases in functions and connect them with the pipe operator, but I don’t know if like a good approach or counterproductive

1 Like
7

It’s not optimizing so much, but returning an error tuple that indicates which step failed would be an improvement - I like the way it laid out in this gist by Devon Estes.

On the other hand you could also just not return a tuple at all; I think generally when functions end in ? that is not the expected type of return value.