Calculating subtotals on a list of maps

Questions / Help
1

Hi,

I’d appreciate any advice on the code below. I’m calculating subtotals by groups of products. The products are in an unsorted list of maps. I’m calling my own subtotal function in Enum.map because some of the groups require special treatment. I’ve included one example of such a treatment.

This works, but before writing another 10 of these “treatments” I want to make sure I have the basic structure right. Also, the idea is to return the results in JSON specifying the groups, subtotals, and total. Anything I should do or not do to facilitate that?

  def subtotals_by_group(list_of_products) do
    list_of_products
    |> Enum.group_by(fn %{group_number: g} -> g end, fn %{price: p} -> p end)
    |> Enum.map(fn {group, prices} -> subtotal(group, prices) end)
  end

  def subtotal("PROMO1", prices) do
    count = Enum.count(prices)

    cond do
      count < 5 ->
        Enum.sum(prices)
      count >= 10 ->
        additional = count - 10
        {:ok, ["PROMO1", (20 + additional * 4) / 1]}
      true ->
        {:error, "Invalid choice"}
    end
  end

  def subtotal(group, prices) do
    {:ok, [group, Enum.sum(prices)]}
  end

Thanks in advance

1 Like
2

Looks good to me! :+1:

Just a couple of minor things I’d do differently:

  1. Replace
    |> Enum.group_by(fn %{group_number: g} -> g end, fn %{price: p} -> p end)
    with
    |> Enum.group_by(& &1.group_number, & &1.price)
  2. Return {group, sum} instead of [group, sum]
3 Likes
3

Thank you. This is exactly what I was looking for. I have read a lot, but now I need practice to put it all together. I’m sure I read somewhere that tuples is the preferred internal representation but I guess I forgot about it.

4

Actually, thanks to you I found a minor bug in my code. Before, when passing less than 5 products to the special treatment function I was just returning the price without the group. Now, I fixed it like this:

def subtotal(“PROMO1”, prices) do
count = Enum.count(prices)

cond do
  count < 5 ->
    {:ok, {"PROMO1", Enum.sum(prices)}}

  count >= 10 ->
    additional = count - 10
    {:ok, {"PROMO1", (20 + additional * 4) / 1}}

  true ->
    {:error, "Invalid choice"}
end

end

So, thanks again :smiley:

1 Like
5

You’re welcome!

6

I might even simplify it a little further to:

|> Enum.map(&subtotal/1)

And change your subtotal/2 definition to subtotal/1:

def subtotal({"PROMO1", prices}) do
  # implementation
end

def subtotal({group, prices}) do
  # implementation
end

But that doesn’t really make too much of a difference since most of the code will be in all the subtotal function clauses.

1 Like
8

You’re right. It won’t make much of a difference, but you have showed me another (more elegant) way to accomplish the same thing. I love it. Big thanks!

1 Like
9

@Cruz, if it’s not too much trouble, would you mind posting some sample input / output?

I’m a beginner and your code looks like it would be fun to figure out.

I just need that little nudge. :thinking:

1 Like
10

No problem at all man. Just allow me a day to get back to you because I already closed for today :wink:

1 Like
11

Awesome; thanks!

And I think I’ll switch gears a bit on my approach; instead of working through your code I’ll start with the input/output, see if I can come up with a solution (no peeking, I promise :innocent:), and then compare.

This’ll be fun!

12

Is it necessary to write this formula this way? What about

(4*count - 20) / 1

And the dividing by 1 is from reason converting the price to float value? If yes I would use :erlang.float(4*count - 20)

1 Like
13

Wait? Prices? Float? This sounds wrong!

The usual top 3 from google, never read them, but they are propably correct, their headline seems to be at least:

1 Like
14

To link what you probably want to use with some more infos about why you should use a proper money type: https://github.com/kipcole9/money#falsehoods-programmers-believe-about-prices

3 Likes
15

Good point :+1: Thank you for the posted links.

1 Like
16

@ondrej-tucek

You’re right on both points. The formula can be simplified, and I was doing /1 to show floats. I guess I can also do:

4.0 * count - 20.0

That’s simple and makes the intention clear.

@NobbZ @LostKobrakai

Thank you for pointing that out. I’ll switch to the Elixir money library if this PoC goes somewhere.

@ All

Just out of curiosity, I guess Elixir follows Erlang on not allowing a binary to float conversion if the input binary is written as an integer. Any idea why the Erlang team made that decision in the first place?

2 Likes