Elegant refactory of a function

Questions / Help
,
1

I’ve got this function, translated from an old codebase (imperative language) into Elixir, by our contractor.

  def pub0(payload) when is_map(payload) do
    if ["country", "commodity", "share", "mon", "cog", "esa1"] |> Enum.all?(fn x -> Map.has_key?(payload, x) end) &&
        Map.get(payload, "esa1") == "hs10" &&
        is_intger(Map.get(payload, "cog") do
      payload = Map.put(payload, "itime", utimestamp(2))
      emporio_ts(Jason.encode!(payload))
    end
  end

The function works fine, it performs its job, but I don’t like the code structure:

There is an if with tree conditions that have to evaluate all to true (logically: if (true & true & true) then …). If satisfices this, then adds new property to the object and finally calls another function.

It does not seem functional. Is there a way to refactory the function in a (more) elegant and functional manner? And also more comprehensible, consistent, shorter ?

Thanks! :smiley:

2

Personally, I’d probably go for this:

def pub0(%{"country" => _, "commodity" => _, "share" => _, "mon" => _, "cog" => cog, "esa1" => "hs10"} = payload) when is_integer(cog) do
    payload
    |> Map.put("itime", utimestamp(2))
    |> Jason.encode!
    |> emporio_ts()
  end
  
def pub0(_), do: nil
5 Likes
3

The usual micro-scale cleanups could help (extract the parts of that giant conditional into functions with meaningful names, for instance) but the shape-checking on payload makes me wonder if it should be a struct.

That would require much larger changes to the calling code, but simplify pub0 considerably.

4

This looks very cool, but it fails: (MatchError) no match of right hand side value.
Maybe because the payload has much more properties than those six tested for. The rest are not mandatory but in total are ~30 properties.
Actually I learnt that (in prod) the number of props of the payload is variable, between 20 and 35.

5

No, maps (thankfully) support partial matches :slight_smile:

Share your code and the exact map you’re passing to pub0, I’m sure we can figure out what’s going on

6

I’ve learnt map support partial matches but still it fails with that error.
Unfortunately, I can’t share more code/data (for legal reasons), I have to debug it myself.

1 Like
7

It can’t fall with that catch-all returning nil …?

1 Like