# Elixir - Points bot using Ecto

I'm making a bot that will be used for "points" tracking and such, like a reward system and I'm using Ecto with postgrex, this is my approach so far, it works but I'm not sure if I'm doing it in an "Elixir", functional way or if I can improve this a bit ^-^

defmodule Cord.Commands do
...
case Repo.get_by(Rewards, user: msg.author.id) do
# Adds user if it doesn't exist
nil ->
case Repo.insert(%Rewards{user: msg.author.id}) do
{:ok, _struct} ->
Api.create_message(msg.channel_id, ":white_check_mark:")
{:error, _changeset} ->
Api.create_message(msg.channel_id, ":x:")
end
_ ->
end
end

# Maps to each id in the list and updates each one
def thanks(msg) do
Enum.map msg.mentions, fn user ->
repo_user = Repo.get_by(Rewards, user: user.id)
points = repo_user.quantity
repo_user = Ecto.Changeset.change(repo_user, quantity: points + 1)
Repo.update!(repo_user)
end
end

def not_found(_), do: IO.inspect :ignore
end


Here are a few suggestions...

You could DRY up your code in add_me by returning the string and then making the API call in one place. Also, you should call Repo.insert/1 with a changeset. This will ensure your validation is checked. You can also remove the nesting with the with special form. Its a really handy function that I recommend you learn if haven't already.

  def add_me(msg) do
message =
with nil <- Repo.get_by(Rewards, user: msg.author.id),
changeset <- Rewards.changeset(%Rewards{}, %{user: msg.author.id}),
{:ok, _} <- Repo.insert(changeset) do
":white_check_mark:"
else
{:error, _} -> ":x:"
end
Api.create_message(msg.channel_id, message)
end


You can reduce some of the one-time variables in thanks like this:

  def thanks(msg) do
Enum.map msg.mentions, fn user ->
repo_user = Repo.get_by(Rewards, user: user.id)

repo_user
|> Ecto.Changeset.change(quantity: repo_user.points + 1)
|> Repo.update!
end
end


Finally, it appears that you don't care about the type of error on the Repo.insert. Keep in mind that the changeset returned with {:error, changeset} contains the validation errors.

NOTE:

I see 4 common style / issues with programmers learning Elixir.

1. error Calling a side affect free function without using the return value. Data in Elixir is immutable, so you need need to either capture the results of a function, call it as the last statement so it returned to the caller, or create a side effect in the function (send an event, write to database, file, etc)
2. warning Use a variable outside the scope of the block it was set in. In other words, don't set a variable inside an if, case, or cond, then use it later
3. style Use lots of single use variables. This is typically a sign that the pipe |> operator may clean up the code.
4. style Use logs of conditional statements where multi-clause function heads with pattern matching and guards may make the code cleaner, with less nesting
• Hello Steve, thank you very much, I have some questions regarding this solution that I hope you can clear out. 1st. Do I have to use a Changeset even if I don't want a validation? Because when I insert a value the user doesn't input any information, I'm getting it from a struct attached to the user automatically, the only thing the user does is call the command. 2nd. I've read about with before but honestly I didn't quite understand it so I didn't know it could be used in this case, does it work like a nest case statement or something else? I will look further into it and... – Aguxez May 13 '17 at 0:31
• ... 3rd. I don't actually care (in this case) for the validation errors on Repo.insert, because how I said, I'm not validating any data or I may be wrong? So, should I explicitly check the changeset returned with {:error, changeset} when I'm validating the data? Again, thanks for the help. – Aguxez May 13 '17 at 0:33
• No, you don't need to pass Repo.insert a changeset. Your code will work fine. If you don't care about validation errors then you don't need to check. If you do care about validation errors, then changeset.errors will give you the list. Finally, when I first started using the with operator, I found it a little confusing and hard to remember. However, after using it for a while, its very easy to use and a very nice tool to reduce nesting and handling errors in the middle of a pipeline. – Steve Pallen May 13 '17 at 0:44