Code Review for a sales tax problem solved using Elixir

Questions / Help
8

Your problem is what dialyzer is telling you: you’re not allowed to call initialize_cart_product/2 with (number(), Item) because the spec says it accepts (number(), input_item()).

Indeed, if you look at the definition of input_item/0 in shopping_cart.ex and compare to the item/0 in receipt_csv_parser.ex (which is what you’re passing to initialize the cart product), you’ll see they don’t match (their price and quantity types differ).

Some general tips:

  • you should put all of your modules within a top-level namespace unless you have a good reason not to do so (e.g. Item should be SalesTax.Item). Alias them where they’re used if you’re worried about long module names. Otherwise, any project using your code is going to have a conflict if they (e.g.) also define an Item struct. See here. Note that nothing forces you to do it, but it’s a good convention to follow.

  • you should probably use String.t instead of binary: they’re the same to analysis tools, but String.t makes it more obvious what you’re working with.

  • you’re sprinkling typing information all over your code. This makes it hard to understand and (as you’re finding out) hard to maintain.

For your specific case, I would suggest doing this:

# item.ex
@type t :: %__MODULE__{
    :basic_sales_tax_applicable => boolean(),
    :imported => boolean(),
    :name => binary(),
    :price => number(),
    :quantity => integer()
}

Then, everywhere else (e.g. here) use that value in your specs:

  @spec initialize_cart_product(number, Item.t()) :: Item.t()
  def initialize_cart_product(total_sales_tax_from_one_item, product) do

If you want to differentiate items before/after processing, you can declare additional “sub-types” in item.ex, e.g.:

# item.ex
@type t :: before_processing | after_processing

@type before_processing :: %__MODULE__{
    # type info
}

@type after_processing :: %__MODULE__{
    # type info
}
2 Likes