7
\$\begingroup\$

This is some Erlang code I wrote to output a string of all the IP addresses of the machine. The string is simply printed to the user for him to read.

-module(ip).
-export([get_ip_address_string/0]).

get_ip_address_string() ->
    {ok, IPAddress} = inet:getif(),
    filter_ip_address(IPAddress).

filter_ip_address(IPAddress) ->
    filter_ip_address(IPAddress, []).

filter_ip_address([], Acc) ->
    Acc;
filter_ip_address([{{127,0,0,1}, _Broadcast, _Mask} | Tail], Acc) ->
    %Do not include the loopback address.
    filter_ip_address(Tail, Acc);
filter_ip_address([Head | Tail], Acc) ->
    {{Oct1, Oct2, Oct3, Oct4}, _Broadcast, _Mask} = Head,
    IPString = io_lib:format("~b.~b.~b.~b ", [Oct1, Oct2, Oct3, Oct4]),
    filter_ip_address(Tail, IPString ++ Acc).

Comments on how to write this in a more Erlang way, make it more readable, or anything you would do different are welcome.

\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

First of all your variable IPAddress is misnamed. It sounds as if it contained a single IP address, but in fact it contains a list of multiple triples, where each one does not only contain an IP address, but also the corresponding broadcast address and netmask. You should probably call it something like IPTriples.

If you look at your code it's basically doing three things:

  1. Filtering out localhost
  2. Replacing each ip address with a string representation
  3. Concatenating the thus created list of strings, separating them with spaces.

For each of these 3 steps there's already an erlang function, which can be used to do it:

  1. lists:filter which takes a predicate and a list and returns a list only containing elements that match this predicate.
  2. lists:map which takes a function and a list and returns a list containing the result of the function for each element.
  3. string:join which takes a list of strings and a separator and returns a string with each item in the list separated by the separator.

Usually it it is good style to reuse existing functionality as much as possible instead of reimplementing it. So using these functions, your get_ip_address_string can be rewritten to something like this:

get_ip_address_string() ->
    {ok, IPTriples} = inet:getif(),
    FilteredIPTriples = lists:filter(fun is_not_localhost/1), IPTriples),
    IPAddresses = lists:map(fun ip_triple_to_string/1, FilteredIPTriples),
    string:join(IPAddresses, " ").

is_not_localhost({{127,0,0,1}, _Broadcast, _Mask}) -> false;
is_not_localhost({_IP, _Broadcast, _Mask})         -> true.

ip_triple_to_string({{Oct1, Oct2, Oct3, Oct4}, _Broadcast, _Mask}) ->
    io_lib:format("~b.~b.~b.~b", [Oct1, Oct2, Oct3, Oct4]).

Note that this has also the benefit that the logic for filtering out localhost, the logic for turning the IP addresses into strings and the logic for building the result string from that are now no longer intertwined, which should make the code more manageable.

\$\endgroup\$
6
\$\begingroup\$

My solution is close to sepp2k's:

First a simpler conversion routine:

ip_triple_to_string(Tup) ->
    io_lib:format("~b.~b.~b.~b", tuple_to_list(Tup)).

Then the meat:

get_ip_address_string() ->
    {ok, IPTriples} = inet:getif(),

Not much you can do here. You have to assert that you really got data from the getif/0 call.

    Strings = [ip_triple_to_string(IP) || {IP, _, _} <- IPTriples,
                IP =/= {127,0,0,1}],
    string:join(Strings, " ").

The list comprehension captures the map and filter in one go over the list. Otherwise, it is much like the other definition.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.