CODEWITHSUNDEEP
ectoelixir
BeniaminoBaggins
BeniaminoBaggins

Reputation: 12503

Elixir 'if' and 'and' not working as expected

I have this code:

if Map.has_key?(dbShop, "id") && Map.has_key?(dbProduct, "id") do
  case Api.Repo.insertProductShop(conn, dbShop.id, dbProduct.id) do
    {:ok, productShop} ->
      {:ok, productShop}
    {:error, changeset} ->
      Tuple.append(errors, "could not insert product in the Shop")
  end
else
  IO.puts("(dbShop, id) && Map.has_key?(dbProduct, id) failed")
  IO.inspect(dbShop)
  IO.inspect(dbProduct)
end

Code execution makes it into the the else clause and the logs this to the console:

(dbShop, id) && Map.has_key?(dbProduct, id) failed
%Api.Shop{__meta__: #Ecto.Schema.Metadata<:loaded, "shops">, id: 23,
 latitude: -36.846691, longitude: 174.7745803, name: "Yard Bar & Eatery",
 placeId: "ChIJp6DGbAdIDW0RcbnExyPHvCk"}
%Api.Product{__meta__: #Ecto.Schema.Metadata<:loaded, "products">,
 brand: "baba", description: " zhzngshshhshs", id: 34, image: "no-image",
 name: "Nsn", numberOfVotes: nil, rating: nil}

So we can see that dbShop and dbProduct both have an id and somehow the code execution never makes it into the if clause. What am I doing wrong with my if clause? I want check that they both have an id and if so, go inside the if clause.

Full function in router:

  post "/products" do
    errors = {}
    postedProduct = conn.body_params
    dbProduct = %{}
    dbShop = %{}
    case Api.Repo.insertProduct(conn, postedProduct) do
      {:success, product} ->
        dbProduct = product
        case Api.Repo.insertProductCategories(conn, postedProduct, dbProduct.id) do
          {:ok, categories} ->
            {:ok, categories}
          {:error, failed_operation, failed_value, changes_so_far} ->
            Tuple.append(errors, "could not insert productCategories. Product already has that category")
        end
      {:error, changeset} ->
        IO.puts("product not inserted")
        Tuple.append(errors, "could not insert product. Product already existed")
        IO.inspect(errors)
    end

    if Map.has_key?(postedProduct, "shop") do
      case Api.Repo.insertShop(conn, postedProduct["shop"]) do
        {:ok, shop} ->
          dbShop = shop
          if Map.has_key?(dbShop, :id) && Map.has_key?(dbProduct, :id) do
            case Api.Repo.insertProductShop(conn, dbShop.id, dbProduct.id) do
              {:ok, productShop} ->
                {:ok, productShop}
              {:error, changeset} ->
                Tuple.append(errors, "could not insert product in the Shop")
            end
          else
            IO.puts("(dbShop, id) && Map.has_key?(dbProduct, id) failed")
            IO.inspect(dbShop)
            IO.inspect(dbProduct)
          end
        {:error, changeset} -> # shop already exists
          # Tuple.append(errors, "could not insert shop")
          if Map.has_key?(dbShop, "id") && Map.has_key?(dbProduct, "id") do
            case Api.Repo.insertProductShop(conn, dbShop.id, dbProduct.id) do
              {:ok, productShop} ->
                {:ok, productShop}
              {:error, changeset} ->
                Tuple.append(errors, "The product has already been added to the shop")
            end
          end
      end
    end

    if tuple_size(errors) > 0 do
      IO.puts("errors")
      IO.inspect(errors)
      conn
        |> put_resp_content_type("application/json")
        |> send_resp(200, Poison.encode!(%{
            successs: "success",
            errors: errors
        }))
    else 
      conn
        |> put_resp_content_type("application/json")
        |> send_resp(200, Poison.encode!(%{
            successs: "success"
        }))
    end
  end

Upvotes: 0

Views: 128

Answers (2)

Aleksei Matiushkin
Aleksei Matiushkin

Reputation: 121020

Using if in Elixir is a code smell and a hint that the code might be re-written in more explicit manner. Here I would go with Kernel.SpecialForms.with/1, and instead of:

if Map.has_key?(dbShop, :id) && Map.has_key?(dbProduct, :id) do
  # true
else
  # false
end

use:

with %{id: shopId} when not is_nil(shopId) <- dbShop,
     %{id: prodId} when not is_nil(prodId) <- dbProduct do
  # true
else
  ^dbProduct -> IO.puts "prodId is nil"
  ^dbShop -> IO.puts "shopId is nil"
end

Upvotes: 1

Dogbert
Dogbert

Reputation: 222448

Your map has an atom :id as key, not the string "id", so your if should be:

if Map.has_key?(dbShop, :id) && Map.has_key?(dbProduct, :id) do

If the value is guaranteed to be non-nil if present, you can also shorten this to:

if dbShop[:id] && dbProduct[:id] do

Accessing a value using the bracket syntax will not throw an error if the key is not present, and instead return nil, which is a falsy value in Elixir.

Upvotes: 1

Related Questions