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!