rmoretto

rmoretto

Hello!

So I currently have the following problem: I want to take a map and “sanitize” all the integers fields. Basically verifying if that field value is really an int and replace the parsed value in the map, furthermore, if the verified value is not an valid int it should save nil to the field, for example:

Input Map

%{
    int_a: "1",
    int_b: "not_a_int!"
%}

Output Map:

%{
    int_a: 1,
    int_b: nil
%}

On the system that I am currently working I know that all the integers fields in the map will start with the int_ prefix (important to say that I don’t have full access to the struct of the map, so I can’t write a function that uses a specific field) so I wrote the following code:

def parse_map_int_fields(map) do
    ints_map =
      map 
      # Filter all the Integers field
      |> Enum.filter(fn {k, _v} -> Atom.to_string(k) |> String.starts_with?("int_") end)
      # Remove all fields that are nil or ints
      |> Enum.filter(fn {_k, v} -> !is_integer(v) and !is_nil(v) end)
      # Parse the values to int, returning nil if invalid
      |> Enum.map(fn {key, val} ->
        case Integer.parse(val) do
          :error ->
            {key, nil}

          {parsed, _} ->
            {key, parsed}
        end
      end)
      |> Map.new()
    
    # Merge the original map with the "sanitized" map
    Map.merge(map, ints_map)
  end

Another point is that this code will be ran in a lot of maps (5 to 10 millions maps) with on average 15 fields per map, so I have two questions:

  1. Most important, can this code be simplified? From my perspective it look a little bit convoluted, I would appreciate any tips!
  2. How one would optimize this function? I have wrote a simple benchmark using Benchee that gave me the following results:
Operating System: Linux
CPU Information: AMD Ryzen 7 2700X Eight-Core Processor
Number of Available Cores: 16
Available memory: 15.63 GB
Elixir 1.11.2
Erlang 23.2.3

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
parallel: 1
inputs: none specified
Estimated total run time: 7 s

Benchmarking parse_map_int_fields...

Name                           ips        average  deviation         median         99th %
parse_map_int_fields      857.59 K        1.17 μs  ±2149.16%        1.01 μs        2.12 μs

Code used for the benchmark:

defmodule Sanitizer do
  def parse_map_int_fields(map) do
    ints_map =
      map
      |> Enum.filter(fn {k, _v} -> Atom.to_string(k) |> String.starts_with?("int_") end)
      |> Enum.filter(fn {_k, v} -> !is_integer(v) and !is_nil(v) end)
      |> Enum.map(fn {key, val} ->
        case Integer.parse(val) do
          :error ->
            {key, nil}

          {parsed, _} ->
            {key, parsed}
        end
      end)
      |> Map.new()

    Map.merge(map, ints_map)
  end
end

test_map = %{
  int_a: "1",
  int_b: "not_a_int!",
  str_a: "This is a string field",
  str_b: "Another string field",
  fl_a: 0.0
}

Benchee.run(%{
  "parse_map_int_fields" => fn -> Sanitizer.parse_map_int_fields(test_map) end
})

Which isn’t all that bad for my use case, but in the interest of learning I would like to know if something could be done different.

Thanks to all! :smiley:

Showing Posts 1 to 10

kokolegorille

kokolegorille

You could simplify in one pass…

|> Enum.filter(fn {k, v} -> cond1(k) and cond2(v) end)

In fact, You could probably do all in one pass. And You should not filter, if later You will merge the filtered {k, v}

Maybe something like this, one iteration only…

Map.new(map, fn {_k, _v}=el ->
  if has_to_be_sanitized?(el), do: sanitize(el), else: el
end)
#
defp has_to_be_sanitized?({k, v}) do
  # check as in your filters...
end

defp sanitize({k, v}) do
  # ...
end

You can even replace with a private function this part.

defp maybe_reshape(el) do
  if has_to_be_sanitized?(el), do: sanitize(el), else: el
end

# This allows You to reduce to
Map.new(map, &maybe_reshape(&1))

You should not hesitate to split your pipeline into small, composable functions.

chungwong

chungwong

You can give this a try, it is using tail recursion but not fully tested

  def parse(m) when is_map(m), do: parse(Map.to_list(m), [])
  def parse([], acc), do: Enum.into(acc, %{})
  def parse([{k, v} | t], acc), do: parse(t, prepend({to_string(k), v}, acc))

  def prepend({"int_" <> _ = k, v} ,acc) when is_binary(v), do: [{String.to_atom(k), to_integer(Integer.parse(v))} | acc]
  def prepend({k, v}, acc), do: [{String.to_atom(k), v} | acc]              

  def to_integer(:error), do: nil 
  def to_integer({parsed, _}), do: parsed                       

  Benchee.run(%{
    "parse" => fn -> Sanitizer.parse(test_map) end
  })
ericgray

ericgray

Another option is to use reduce. You can map the key/value pairs into a new map to get the desired result. You can also break your code out into smaller functions that do one thing to make your code more readable. Many different ways to do it. Just a matter of taste.

  def parse_map_int_fields(map) do
    Enum.reduce(map, %{}, &do_parse_map_int_fields/2)
  end

  # Reconstruct or accumulate key value pairs into a new map
  def do_parse_map_int_fields({key, value}, map) do
    case starts_with_int?(key) do
      true ->
        Map.put( map, key, parse_value(value) )

      false ->
        Map.put( map, key, value )
    end
  end

  # Boolean function returns true if field is int
  def starts_with_int?(key) do
    key
    |> Atom.to_string()
    |> String.starts_with?("int_")
  end

  # Parses the value of int field, if not an integer return nil
  def parse_value(value) do
    case Integer.parse(value) do
      {value, _} -> value
      :error -> nil
    end
  end
amnu3387

amnu3387

Reduce is fine and a single iteration:

@spec reshape_map_int_fields(map) :: map
def reshape_map_int_fields(map) when is_map(map),
  do: Enum.reduce(map, %{}, fn {k, v}, acc ->
        Map.put(acc, k, maybe_reshape(k, v))
      end)

@spec maybe_reshape(atom, term) :: integer | term
def maybe_reshape(k, v) when is_atom(k) and not is_integer(v) and not is_nil(v) do
  case Atom.to_string(k) do
    <<"int_", _>> ->
      case Integer.parse(v) do
        {new_val, _} -> new_val
        _ -> nil
      end
    _ ->
      v
  end
end

def maybe_reshape(_, v), do: v
gregvaughn

gregvaughn

I haven’t benchmarked, but you could do something with a for comprehension

iex(103)> map = %{
...(103)>     int_a: "1",
...(103)>     int_b: "not_a_int!",
...(103)>     other: "leave me alone"
...(103)> }
%{int_a: "1", int_b: "not_a_int!", other: "leave me alone"}
iex(104)> for {k, v} <- map, match?("int" <> _, Atom.to_string(k)), into: map do
...(104)>   case Integer.parse(v) do
...(104)>     {int, _} -> {k, int}
...(104)>     _other -> {k, nil}
...(104)>   end
...(104)> end
%{int_a: 1, int_b: nil, other: "leave me alone"}
rmoretto

rmoretto OP

Thanks people for all the responses!

Indeed a lot of way to approach the problem, I will probably use the @amnu3387 response just because is a little bit faster, but as Eric said, is just a matter of taste, and all other options would be completely fine.

Thanks again for the help!

The benchmark results were:

Name                             ips        average  deviation         median         99th %
mnussbaumer_response        927.74 K        1.08 μs  ±2239.93%        0.96 μs        1.98 μs
kokolegorille_response      867.51 K        1.15 μs  ±2327.39%        1.00 μs        2.14 μs
gregvaughn_response         766.18 K        1.31 μs  ±2168.73%        1.15 μs        2.31 μs
ericgray_response           730.43 K        1.37 μs  ±1314.86%        1.26 μs        2.48 μs
chungwong_response          655.58 K        1.53 μs  ±1555.15%        1.36 μs        2.78 μs
parse_map_int_fields        567.61 K        1.76 μs  ±1491.38%        1.57 μs        3.18 μs

Comparison:
mnussbaumer_response        927.74 K
kokolegorille_response      867.51 K - 1.07x slower +0.0748 μs
gregvaughn_response         766.18 K - 1.21x slower +0.23 μs
ericgray_response           730.43 K - 1.27x slower +0.29 μs
chungwong_response          655.58 K - 1.42x slower +0.45 μs
parse_map_int_fields        567.61 K - 1.63x slower +0.68 μs

And the code used for the benchmark:

defmodule Sanitizer do
  def parse_map_int_fields(map) do
    ints_map =
      map
      |> Enum.filter(fn {k, _v} -> Atom.to_string(k) |> String.starts_with?("int_") end)
      |> Enum.filter(fn {_k, v} -> !is_integer(v) and !is_nil(v) end)
      |> Enum.map(fn {key, val} ->
        case Integer.parse(val) do
          :error ->
            {key, nil}

          {parsed, _} ->
            {key, parsed}
        end
      end)
      |> Map.new()

    Map.merge(map, ints_map)
  end

  def kokolegorille_response(map) do
    Map.new(map, fn {_k, _v} = el ->
      if has_to_be_sanitized?(el), do: sanitize(el), else: el
    end)
  end

  defp has_to_be_sanitized?({k, v}) do
    Atom.to_string(k) |> String.starts_with?("int_") and !is_integer(v) and !is_nil(v)
  end

  defp sanitize({k, v}) do
    case Integer.parse(v) do
      :error ->
        {k, nil}

      {parsed, _} ->
        {k, parsed}
    end
  end

  def chungwong_response(map) do
    parse(map)
  end

  def parse(m) when is_map(m), do: parse(Map.to_list(m), [])
  def parse([], acc), do: Enum.into(acc, %{})
  def parse([{k, v} | t], acc), do: parse(t, prepend({to_string(k), v}, acc))

  def prepend({"int_" <> _ = k, v}, acc) when is_binary(v),
    do: [{String.to_atom(k), to_integer(Integer.parse(v))} | acc]

  def prepend({k, v}, acc), do: [{String.to_atom(k), v} | acc]

  def to_integer(:error), do: nil
  def to_integer({parsed, _}), do: parsed

  def ericgray_response(map) do
    Enum.reduce(map, %{}, &do_parse_map_int_fields/2)
  end

  # Reconstruct or accumulate key value pairs into a new map
  def do_parse_map_int_fields({key, value}, map) do
    case starts_with_int?(key) do
      true ->
        Map.put(map, key, parse_value(value))

      false ->
        Map.put(map, key, value)
    end
  end

  # Boolean function returns true if field is int
  def starts_with_int?(key) do
    key
    |> Atom.to_string()
    |> String.starts_with?("int_")
  end

  # Parses the value of int field, if not an integer return nil
  def parse_value(value) do
    case Integer.parse(value) do
      {value, _} -> value
      :error -> nil
    end
  end

  def mnussbaumer_response(map) do
    reshape_map_int_fields(map)
  end

  @spec reshape_map_int_fields(map) :: map
  def reshape_map_int_fields(map) when is_map(map),
    do:
      Enum.reduce(map, %{}, fn {k, v}, acc ->
        Map.put(acc, k, maybe_reshape(k, v))
      end)

  @spec maybe_reshape(atom, term) :: integer | term
  def maybe_reshape(k, v) when is_atom(k) and not is_integer(v) and not is_nil(v) do
    case Atom.to_string(k) do
      <<"int_", _>> ->
        case Integer.parse(v) do
          {new_val, _} -> new_val
          _ -> nil
        end

      _ ->
        v
    end
  end

  def maybe_reshape(_, v), do: v

  def gregvaughn_response(map) do
    for {k, v} <- map, match?("int" <> _, Atom.to_string(k)), into: map do
      case Integer.parse(v) do
        {int, _} -> {k, int}
        _other -> {k, nil}
      end
    end
  end
end

test_map = %{
  int_a: "1",
  int_b: "not_a_int!",
  str_a: "This is a string field",
  str_b: "Another string field",
  fl_a: 0.0
}

Benchee.run(%{
  "parse_map_int_fields" => fn -> Sanitizer.parse_map_int_fields(test_map) end,
  "kokolegorille_response" => fn -> Sanitizer.kokolegorille_response(test_map) end,
  "chungwong_response" => fn -> Sanitizer.chungwong_response(test_map) end,
  "ericgray_response" => fn -> Sanitizer.ericgray_response(test_map) end,
  "mnussbaumer_response" => fn -> Sanitizer.mnussbaumer_response(test_map) end,
  "gregvaughn_response" => fn -> Sanitizer.gregvaughn_response(test_map) end
})
ericgray

ericgray

I like this approach. Simple elegant solution. Never thought about match?

eksperimental

eksperimental

you can optimize it even more:
def maybe_reshape(k, v) when is_atom(k) and is_binary(v) do

eksperimental

eksperimental

Additionally, IIRC correctly it could be faster to reduce into a keyword list and convert that to a map with :maps.from_list. You iterate twice the list though. I would like to see the benchmarks.

amnu3387

amnu3387

Yeap, you beat me to it. I think that if I was writing this I would probably do:

@spec reshape_map_int_fields(map) :: map
def reshape_map_int_fields(map) when is_map(map),
  do: Enum.reduce(map, %{}, fn {k, v}, acc ->
        Map.put(acc, k, maybe_reshape(k, v))
      end)

@spec maybe_reshape(atom | String.t, term) :: integer | nil | term
def maybe_reshape(k, v) when is_atom(k) and is_binary(v),
  do: maybe_reshape(Atom.to_string(k), v)

def maybe_reshape(<<"int_", _>>, v) when is_binary(v) do
  case Integer.parse(v) do
     {new_val, _} -> new_val
      _ -> nil
  end
end

# perhaps add a special clause for when it's still `"int_"` but not binary, not nil
# and not integer to error out, or nillify but that would depend

def maybe_reshape(_, v), do: v

This would have the benefit of also converting binary keys besides atom ones.

Where Next? Top

Trending in Questions Top

stjefim
Hello! Suppose you are building workflow (order / task / payment) processing system with the following requirements: Each workflow con...
New
itzmidinesh
Hi all, I’m working on a Phoenix project where I need to make external HTTP requests and I’m trying to choose between Req and HTTPoison....
New
pragdave
How do I tell Hologram not to escape HTML when substituting into templates? {Textile.render(@product.long_description)} escapes the HTM...
New
rlipscombe
I’d like to create a file with 600 permissions (rw-------), but there’s a race condition if I create it and then change the permissions. ...
New
leoschweizer
Hi, we run different LLM models across a cluster, with each node loading a configurable subset. Model-specific queues let nodes consume ...
New
rlipscombe
When I use Mix.install with a git dependency, using git@github.com:... how do I change the SSH identity used? I’ve got two Github accoun...
New
adr
Hello all. I have a question as to best practices for code organization when implementing a liveview with a lot of responsibilities. Sup...
New

Other Trending Topics Top

dominicletz
Hey there, I haven’t seen a list of useful Elixir specific skills for the popular AI tools yet so I’m starting one here with my first. ...
New
raa-ziq
Hi everyone :waving_hand: I wanted to share something I’ve been building — NowBlind, a random chat app designed to help people discover ...
New
mudasobwa
I fully migrated to my own harness from Anthropic/Gemini and I think it’s time to share it. Welcome DSH, the DeepSeek Harness, fully writ...
New
KristerV
Hey. Is there anyone here who creates agents in their apps? Not talking about using agents, but creating them. I’m finding it pretty diff...
New
OndrejValenta
(I just needed to vent somewhere and LinkedIn is full of hope, or hype, I’m not sure which exactly) AI dream has many faces, but general...
New
kip
Localize is the next generation localisation library for Elixir. Think of it as ex_cldr version 3.0. The first version will be released ...
New

Latest on Elixir Forum

We're in Beta

About us Mission Statement

Options

Thread Display Mode




Thread Preview

Skip Thread Previews