with true <- viewed_user != nil,
true <- current_user != nil,
true <- viewed_user.id == current_user.id && current_user.id != nil,
%{map: map, giver_name: giver_name} <-
AccoladeLib.recent_accolade(current_user.id, days) do
"You have recently received an accolade from #{giver_name} for your match in #{map}!"
else
# Goes here if the viewed user is not the same as the logged in user
# Or if there are no recent accolades
_ -> nil
end
Usually with statements check for :ok. Is it acceptable to check for non nil values like I’m doing here? if not how should I do it?
Personally I would try and avoid being the situation of having to check for nil ids. Make sure you either have a user or you don’t (a struct or nil), otherwise this type of logic gets littered throughout the whole app.
With that, you could do:
with %User{id: id} <- viewed_user,
%User{id: ^id} <- current_user,
%{map: map, giver_name: giver_name} <-
AccoladeLib.recent_accolade(current_user.id, days) do
# ...
else
# ...
end
(you can add a when guard on the first id if you really can’t be sure that it can’t be nil)
Not sure this is exactly how I’d do it but I’d need more context.
Otherwise I would just extract all that user checking to its own function or something.
What’s usually done does not matter too much. Here’s an alternative take but I don’t think it’s better in any way really:
case {viewed_user, current_user} do
{nil, nil} -> #...
{nil, _user} -> #...
{_user, nil} -> #...
{user1, user2} -> # different IDs
{user, user} ->
# viewed and current user are the same.
if not is_nil(user.id) do
case AccoladeLib.recent_accolade(current_user.id, days) do
%{map: map, giver_name: giver_name} -> # success
other -> # error
end
else
IO.puts("ERROR: user ID is nil")
end
end
Additionally, I’ll again go against “the official guidelines” and claim there is nothing wrong in adding tags to your with matchers, in this case:
with {:viewed_user_exists, true} -> {:viewed_user_exists, not is_nil(viewed_user)},
# etc. for the rest
…because in the error clause below you can clearly distinguish each and every error and act on them separately.
Like others have suggested, ideally you could skip some of these checks by not creating user-shaped structs with a nil in id.
Otherwise, pattern-matching might be better. Consider extracting that statement to a private function:
defp recent_accolade_message(%{id: id}, %{id: id}, days) do
case AccoladeLib.recent_accolade(id, days) do
%{map: map, giver_name: giver_name} ->
"You have recently received an accolade from #{giver_name} for your match in #{map}!"
nil ->
nil
end
end
defp recent_accolade_message(_, _, _), do: nil