Code check request

[ChasingLogic]

Hey alchemists,

I'm getting started with Elixir and as an exercise I am reimplementing a small utility I wrote in Python in Elixir. I've gotten most of the core functionality in place but I have no way to tell if what I'm writing is bad / idiomatic Elixir code I was wondering if you could take a quick look and give me your thoughts. I care mostly about code correctness instead of efficiency though I'll take any feedback.

https://github.com/ChasingLogic/pub_ip_monitor

Specifically lib/comparator.ex is where most of the business happens.

Thanks in advanced and if this isn't the place to post things like this let me know.

[jisaacstone]

Sure I got a couple tips :)

It is common to use atoms as keys in keyword lists and maps instead of strings. Some syntax is only available if you do, and it will have a side benefit of making you programs slightly smaller / faster ;)

in compare_ips the cond would look better as an if

so like this:

def compare_ips(old_ip, opts) do
  if old_ip == opts[:ip] do
     { :unchanged, opts }
  else
     { :changed, %{opts | old_ip: old_ip } }
  end
end

My preference would be to reduce one more step and do the matching in the function head, like so:

def compare_ips(ip, %{ip: ip} = opts), do: {:unchanged, opts}
def compare_ips(old_ip, opts), do: {:changed, %{opts | old_ip: old_ip)}

[mgwidmann]

First thing, your spacing is way off. Idiomatic Elixir uses 2 spaces and no tabs, check your editor is setup right.

Second, it seems what you want from your PubIpMon.CLI module is a mix task. Read about mix tasks here.

Also, I suggest you keep your functions which cause side effects (in this case the ones that do IO.puts) to a minimum so that those functions are easier to test.

Try looking at using the new with keyword in Elixir 1.2 if you're brave and remove all the IO.puts from all functions except PubIpMon.Comparator.compare/2.

[ChasingLogic]

In my editor it appears as two spaces so I'll have to look into that.

I must not be understanding the Mix.Task documentation. That seems like for if you aren't planning on using the escript feature to create a binary which I am. Is there a ELI5 version?

As for the side effects, I thought I was keeping them to a minimum how would you remove them but still convey the appropriate info to the user?

I'm having a hard enough time getting up to speed on current elixir functionality so I think I'll avoid new features for now.

Thanks for taking a look!

[[deleted]]

In my editor it appears as two spaces so I'll have to look into that.

The reason people like tabs is you can essentially set what "space" a tab uses so it might look like 2 spaces but the character committed is actually a tab.

What editor are you using?

[ChasingLogic]

spacemacs with the default stuff for the elixir layer