Nefcairon
To improve a function
Hello,
I am still an Elixir newbie. To learn I keep on asking questions like this…
Can I improve this two functions? It feels like but I do not know how.
defp get_url(team_id) when team_id > 0 do
"a_url_removed_because_not_public" <>
Kernel.inspect(team_id)
end
defp get_url(team_id) when team_id == 0 do
"a_url_removed_because_not_public"
end
besides doing them as one-liners:
defp get_url(team_id) when team_id > 0; do: "a_url_removed_because_not_public" <> Kernel.inspect(team_id)
defp get_url(team_id) when team_id == 0; do: "a_url_removed_because_not_public"
It still looks as it contains too much redundant code.
Marked As Solved
Qqwy
Quite possibly!
Here are a couple of questions for you, that might help you:
- Are the URLs in the two function clauses the same? If they are, you could extract this out to either another function, or to a module attribute.
- Instead of using
Kernel.inspect(which is meant to only be used for logging/introspection), you probably want to useto_string, or string interpolation. Also, there is no reason to useKernel.in front of a function, because the Kernel module is always (unless explicitly overridden) imported into a module. - Why have the
get_in the name of the function? Depending on the context, this might matter for the meaning of the function name but maybe it is fluff that does not add extra meaning.
Also Liked
NobbZ
I’d add a guard is_integer(team_id) as any non numeric type is greater than 0.
Also, I’d use string interpolation rather than inspect.
Inspection in general is something I’d only use for logging, never to create a user facing string.
kokolegorille
You might use
defp get_url(0) do ...
Qqwy
This is not required. And if you really want to put a verb in there, get is about as vague as you can be, since it means so many different things
. Using a verb like get or fetch as part of a function name would make more sense if you retrieve the URL value from somewhere else. In this case, where it is something the function itself calculates, I would just leave it out:
defmodule Team do
def url(team_id) do
...
end
end
Here, it is clear that Team.url(foo) will return an URL based on foo that has to do with a ‘Team’.
Of course, naming things quickly enters the realms of personal preference. It is very easy to spend all afternoon bikeshedding about it. It’s possibly the least important suggestion of all of the suggestions that people have given in this topic
.









