Please show me (a noob) a way to good functional thinking

Questions / Help
1

Hello.

I wrote a function following

defmodule Problem.Logic do
  alias Problem.Repo
  import Ecto.Query

  def make_problem_if_not_exist(room_id, local_id, title) do
    get_problem_by_room_and_title(room_id, title)
    |> handle_dup_check_and_make_new_problem_if_not_exist(room_id, local_id, title)
  end
  defp handle_dup_check_and_make_new_problem_if_not_exist(%Problem{}, _room_id, _local_id, _title) do
    {:error, :already_exists}
  end
  defp handle_dup_check_and_make_new_problem_if_not_exist(:nil, room_id, local_id, title) do
    make_new_problem(room_id, local_id, title)
  end
  def make_new_problem(room_id, local_id, title) do
    Problem.changeset(%Problem{}, %{
      room_id: room_id,
      title: title
    })
    |> Repo.insert() #expect {:ok, %Problem}
  end
  def get_problem_by_room_and_title(room_id, title) do
    Repo.one(from p in Problem, where: (p.room_id == ^room_id and p.title == ^title) )
  end  
end

I think i am doing wrong thing…
Would you show me how should be done?
I don’t expect the working code but the functional way of the thinking advise…

Thank you.

2

The best way to good, functional thinking today is Dave’s course. https://pragdave.me/ You’ll be thinking in Elixir in no time at all.

1 Like
3

From a functional point of view it looks ok. However you may have gone a bit function mad there. For instance you could re-write that like :

def make_problem(room_id, local_id, title) do
  case get_problem(:room_and_title, room_id, title) do
    %Problem{} -> {:error, :already_exists}
    :nil -> make_problem(local_id, %{room_id: room_id, title: title})
  end
end

def make_problem(local_id, attrs),
  do: Problem.changeset(%Problem{}, attrs) |> Repo.insert()

# The atom here will let you change the query depending on the scenario you
# are calling the function in
def get_problem(:room_and_title, room_id, title),
  do: Repo.one(from p in Problem, where: (p.room_id == ^room_id and p.title == ^title) )

The only thing I’d say is watch the names of your functions. Since you have pattern matching to work with here (and really good matching at that) we can simplify the names of our functions a lot more. Then match on the params being sent to the function. That way we can have make_problem instead of a number of functions called make_number_with_x.

1 Like
4

I think that add_problem and find_problem would work as well.

(Suffering from OO-fatigue I find myself quite often committing additional effort to find something more expressive than the de facto choices of “get” and “set” - I will use them but only after I convinced myself that I’m not just being lazy).

2 Likes