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
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))
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.