`the variable "url" is unsafe` warning and if statement

In following code I receive the warning:

url = base_url

if path do
  url = URI.merge(url, path)
end

if params do
  url = URI.merge(url, "?" <> URI.encode_query(params))
end
warning: the variable "url" is unsafe as it has been set inside a case/cond/receive/if/&&/||. Please explicitly return the variable value instead. For example:

    case int do
      1 -> atom = :one
      2 -> atom = :two
    end

should be written as

    atom =
      case int do
        1 -> :one
        2 -> :two
      end

I rewrote it as:

url = base_url
url = if path, do: URI.merge(url, path), else: url
url = if params, do: URI.merge(url, "?" <> URI.encode_query(params)), else: url

My first question: “Why it’s considered unsafe?”
And second one: “Is there more elegant ways to handle it?”

In your case it would be safe, since you initialised it in the “global” scope, but if you hadn’t and would it set only inside the ifs bodys, and both conditions were false but you try to read from it at the end, it would BANG!.

Also this way of leaking variables is inconsistent across the different special forms, some do allow it, some not.

These 2 reasons together were enough for the elixir core team to deprecate this so called “imperative assignment” with elixir 1.3.

A more elegant way to to this, were to move both conditional assignements into functions and do some piped operations as in the following example with bad naming:

def foo(base_url, path \\ false, params false) do
  url = base_url
  |> merge_path(path)
  |> merge_params(params)

  # do remaining stuff
end

def merge_path(url, path) when !path, do: url
def merge_path(url, path), do: URI.merge(url, path)

def merge_params(url, params) when !params, do: url
def merge_params(url, params), do: URI.merge(url, "?" <> URI.encode_query(params))
2 Likes

Thank you, good example, now I get it. Can you give more information about deprecation of “imperative assignment” in elixir 1.3? (maybe some links).

I’ve found some discussion here

2 Likes

I can’t find the discussion or parts of it, but it is mentioned in the release notes for 1.3:

1 Like

Besides it being safer to not have ‘imperative’ (stateful) assignments, being sure that no conditional ever modifies the state outside allows certain compiler optimizations which would result in more efficient code.

2 Likes