Ensuring param is not nil inside with statement?

I have the following statement:

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?

EDIT: I can’t read

I need the error handling for this line:

%{map: map, giver_name: giver_name} <-
           AccoladeLib.recent_accolade(current_user.id, days)

since it could return something else.

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.

1 Like

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
2 Likes