Hi, I have this double condition if foo && bar do something and all the other cases basically don’t interest me. both foo and bar are assigns in a liveview socket, and I believe there’s a nicer, more elixir style way, of writing my function. I am thinking of some cond with pattern matching, but not sure how to put together the syntax - I am basically stuck at the very first line of the block :-o Can you please give me a hint? Thank you
# 1. check whether we have socket.assign.user # 2. check if all todos are true IO.inspect socket.assigns socket = if (socket.assigns.user && list_completed?(socket.assigns.list_id)) do email = socket.assigns.user.email list_url = "https://organizer.gigalixirapp.com/#{list_id}" IO.puts "All tasks complete!" socket = socket |> put_flash(:info, "All tasks complete! Sending notification to #{email}...") Task.Supervisor.start_child(Organizer.TaskSupervisor, fn -> IO.puts "Sending to #{email} from within a Task" Organizer.Mailer.send_completion_notification(email, list_url) end) socket else IO.puts "Nope, either no user, or list not complete yet..." socket end ... defp list_completed?(list_id) do Lists.list_todos(list_id) |> Enum.all?(&(&1.done)) # checks if all true https://hexdocs.pm/elixir/Enum.html#all?/2 end
I wouldn’t refactor it. Yes, you could use a case, cond, or with, but I don’t think any of them bring clarity over what you have. You only have 2 cases and your conditions are boolean.
2 Likes
Here’s my take on it:
IO.inspect socket.assigns if (socket.assigns.user && list_completed?(socket.assigns.list_id)) do email = socket.assigns.user.email list_id = socket.assigns.list_id send_completion_notification(email, list_id) IO.puts "All tasks complete!" put_flash(socket, :info, "All tasks complete! Sending notification to #{email}...") else IO.puts "Nope, either no user, or list not complete yet..." socket end # .... defp send_completion_notification(email, list_id) do Task.Supervisor.start_child(Organizer.TaskSupervisor, fn -> IO.puts "Sending to #{email} from within a Task" list_url = "https://organizer.gigalixirapp.com/#{list_id}" Organizer.Mailer.send_completion_notification(email, list_url) end) end
If possible, I like to avoid rebinding (the socket = ...something that uses socket... pattern) partly because it adds indentation and partly because it’s a warning sign of complexity.
Sometimes you can avoid that by shuffling operations; this version starts the task before calling put_flash (versus the original that does the reverse) but there shouldn’t be any observable side-effect.
I broke send_completion_notification out into a private function to keep the main chunk of code focused at a single level of abstraction: manipulating socket's contents.
Bigger tidying that could be done, depending on the context: is there something that ensures socket.assigns.user is set before this code runs? If so, consider skipping the re-check in the if clause and Let It Crash.
1 Like
I like this! Not changing the logic, as @gregvaughn suggested, but nicely broken down into cleaner and more readable parts. Thank you!