1
\$\begingroup\$

I've recently started learning elixir, and the following is my attempt at fetching the local IP address of the system.

It can definitely be improved further, but as this would be my first ever project in a functional language; I'd be open to any criticisms.

defmodule Systemip do def private_ipv4() do {:ok, addrs} = :inet.getif() filtered = Enum.filter( addrs, fn address -> ip = elem(address, 0) is_private_ipv4(ip) end ) elem(hd(filtered), 0) end defp is_private_ipv4(ip) do case ip do {10, _x, _y, _z} -> true {192, 168, _x, _y} -> true {172, 16, _x, _y} -> true {172, 32, _x, _y} -> true _ -> false end end end 

This would be part of a larger project, which is still under construction. The IPs are fetched using the erlang call to :inet.getif, the formatting was done by mix format command.

\$\endgroup\$
1
  • 1
    \$\begingroup\$Very interesting code! Definitely willing to take a crack at this, which I'll do as soon as I get a chance. I deleted an earlier question that was obvious when I read your question properly :)\$\endgroup\$CommentedMay 26, 2021 at 11:42

1 Answer 1

2
\$\begingroup\$

There is few problems there:

  1. There is no :inet.getif/0 function listed in inet module docs, which mean that if such function exists, then it is non-public API and should not be called. You probably wanted to call :inet.getifaddrs/0.
  2. hd/1 will fail if filtered list is empty
  3. You traverse whole list to find only first entry.
  4. In Elixir, by convention, boolean returning functions, that aren't guard safe (so all user-defined boolean returning functions), should use ? suffix instead of is_ prefix.
  5. Use pattern matching instead of elem/2 if possible.
  6. This will raise error if there will be error during fetching interfaces addresses.
  7. Your IP ranges are invalid, as whole 172.16.0.0/12 is private, which is range from 172.16.0.0 to 172.31.255.255 where you were checking only for 172.16.c.d and 172.32.c.d where the second group is in public space.

To fix all the issues there you can write it as:

defmodule SystemIP do def private_ipv4() do with {:ok, addrs} <- :inet.getif() do Enum.find_value(addrs, fn {if_name, if_opts} -> case Keyword.fetch(if_opts, :addr) do {:ok, adr} -> if not public_ipv4?(addr), do: addr _ -> false end end) end end defp private_ipv4?({10, _, _, _}), do: true defp private_ipv4?({172, b, _, _}) when b in 16..31, do: true defp private_ipv4?({192, 168, _, _}), do: true defp private_ipv4?({_, _, _, _}), do: false end 
\$\endgroup\$

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.