How to improve this snippet (piping result of a function into another function) with Mogrify

Questions / Help
1

Hi, hope for any wisdom on how to improve this code snippet or if there are bits of knowledge I appear to be lacking

Basically I want to create a map called data containing a range of persons with names ranging from Person 0-100 with a dynamically generated image generated by Mogrify.

But I see my code is a little unwieldy and will love opinions on how to work on it

Many thanks!

def generate_data do
  for x <- 0..100 do
    {:ok, image_data} = Map.fetch(render_image(x),:buffer)
    data(
      name: "Person #{x}",
      image_data: image_data
      )
  end
end

def render_image(x) do
  import Mogrify
  %Mogrify.Image{}
  |> custom("size", "300x300")
  |> custom("background", "#000000")
  |> custom("gravity", "center")
  |> custom("fill", "white")
  |> custom("pango", ~s(<span foreground="yellow">Good day to you, </span>) <> "#{x}")
  |> custom("stdout", "png:-")
  |> create(buffer: true)
end
2

For what it’s worth I think the code looks fine!

What seems off to you?

3

render_image is going to return a Mogrify.Image struct - it will always have a buffer key, so you can shorten that line to image_data = render_image(x).buffer

1 Like
4

Am a super noob at elixir but very keen to learn as much as I can. What i wondered was perhaps if any further improvements can be made… is there a need to call a function instead of piping results. But guess I have a lot to learn still.

5

Thanks! I learnt something new today! Just wondering, when would be the right time to use Map.fetch? I assume it allows error handling in the event the key is not found?

6

Well you are doing great so far.

Piping is just syntax sugar, using it is not refactoring. What you have is perfectly readable so I think it’s fine.

1 Like
7

Not always error-handling exactly - Map.fetch is really useful when it’s important to your code whether the key is present at all and the value could potentially be nil.

For instance, if you need to distinguish:

without_baz = %{foo: "bar"}
# from
nil_baz = %{foo: "bar", baz: nil}

then you’ll get different results with different functions in Map:

# square-brackets return nil if the key isn't present
nil_baz[:baz] == without_baz[:baz]

# Map.get can return a default
Map.get(without_baz, :baz, "default value") # => "default value"
# vs
Map.get(nil_baz, :baz, "default value") # => nil

# But if there's no "safe" default that won't appear in the source, Map.fetch is better:
Map.fetch(without_baz, :baz) # => :error
# vs
Map.fetch(nil_baz, :baz) # => {:ok, nil}
1 Like