sodapopcan

sodapopcan

Improve this code?

I thought I would make one of these “improve this code” posts even though this problem isn’t really at that interesting (but here we are). I don’t hate my solution but I feel like it could perhaps be a little more readable. I’m not sure. It’s not bad, but maybe it could be better? Maybe there is an existing way to do this? I am aware of breadcrumble_ex but it’s too explicit for my use-case. I’d also like a better name, though not putting that on you all.

So this function takes a URI path (eg. /path/to/some/place) and turns it into a list of tuples suitable for a presenter to blindly turn it into breadcrumbs. Here it is:

defmodule ExampleModule do
  @doc """
  Takes a path and return a list of tuples in the form of:

    [{"segment", "/path/to/segment"}]

  ## Example:

    iex> ExampleModule.segment_path("/path/to/segment")
    iex> [{"path", "/path"}, {"to", "/path/to"}, {"segement", "/path/to/segment"}]

  This is useful for making breadcrumbs.
  """
  def segment_path(path) when is_binary(path) do
    path
    |> String.split("/")
    |> Enum.reverse()
    |> do_segment_path([])
  end

  defp do_segment_path([""], acc), do: acc

  defp do_segment_path([segment | rest] = path, acc) do
    path =
      path
      |> Enum.reverse()
      |> Enum.join("/")

    acc = [{segment, path} | acc]

    do_segment_path(rest, acc)
  end
end

That’s it!

Not too exciting, I know. I mostly like this little problem since this is one of those cases where building a list from the end is actually what we want to do. Of course, this just means we reverse it at the beginning as opposed to the end, and then again each iteration to put each segment in its correct form. That’s the big thing I think I would find confusing about this coming back to it later with no context.

Code-golf answers are welcome, but I’m ultimately looking for readability improvements.

Marked As Solved

al2o3cr

al2o3cr

YMMV as to where this falls on the golf-vs-readability spectrum, but it’s shorter:

def segment_path(s) do
  s
  |> String.split("/", trim: true)
  |> Enum.scan({nil, ""}, fn el, {_, path} ->
    {el, "#{path}/#{el}"}
  end)
end

The tricky part here is that we only care about part of the “accumulator” for Enum.scan.

Alternatively, you could divide the work into clearer parts:

def segment_path(s) do
  s
  |> String.split("/", trim: true)
  |> Enum.scan([""], &[&1 | &2])
  |> Enum.map(&{hd(&1), Enum.join(Enum.reverse(&1), "/")})
end

The scan here builds a list of lists with (reversed) paths:

[["path"], ["to", "path"], ["segment", "to", "path"]]

Then the map converts those into the desired output shape.

Also Liked

msimonborg

msimonborg

Here’s another option!

  def segment_path(path) when is_binary(path) do
    segments = String.split(path, "/", trim: true)

    paths =
      Enum.reduce(segments, [], fn
        segment, [] -> ["/#{segment}"]
        segment, [head | _tail] = whole -> ["#{head}/#{segment}" | whole]
      end)

    Enum.zip(segments, Enum.reverse(paths))
  end
end

What makes this more readable IMHO:

  • Fewer LOC
  • Contained in one function which tells me a clear story
  • Building the paths as we move forward in a visually clear way with interpolation rather than list |> Enum.reverse() |> Enum.join()
  • Only reversing one list at the end
  • Using Enum.reduce/3 which is very familiar to most Elixir devs

You can of course pipe the reduce block into Enum.reverse() instead of inlining it in the zip call, if that’s your preference

As an added bonus this implementation benchmarks 20% faster on my machine than your original :smile:

GPrimola

GPrimola

Another way:

def segment_path(path) do
  path_levels =
    path
    |> String.split("/", trim: true)
    |> length()

  path
  |> breadcrumb()
  |> Stream.iterate(fn
    {segment, segment_path} ->
      parent_path = String.replace_trailing(segment_path, "/#{segment}", "")
      breadcrumb(parent_path)
  end)
  |> Enum.take(path_levels)
  |> Enum.reverse()
end

def breadcrumb(path) do
  segment = path
  |> String.split("/")
  |> List.last()

  {segment, path}
end

The solution with this code unfolds like this:

path = "/some/path/to/nowhere"

1. {"nowhere", "/some/path/to/nowhere"}
2. {"to", "/some/path/to"}
3. {"path", "/some/path"}
4. {"some", "/some"}

Then it’s reversed.

I won’t add reasoning on why (or whether) this is better, but just another implementation. :slightly_smiling_face:

adamu

adamu

Hopefully this is at least par for the course :in_hole:. @Ninigi I’m interested in what you’d use instead of do_ here.

defmodule Example do
  def segment_path(path) do
    path |> String.split("/", trim: true) |> do_segment_path("")
  end

  defp do_segment_path([], _path), do: []

  defp do_segment_path([current | rest], path) do
    new_path = "#{path}/#{current}"
    [{current, new_path} | do_segment_path(rest, new_path)]
  end
end

Running:

iex(1)> Example.segment_path("path/to/segment")
[{"path", "/path"}, {"to", "/path/to"}, {"segment", "/path/to/segment"}]

Last Post!

sodapopcan

sodapopcan

I mean, ya, that one certainly works well :sweat_smile:

Where Next?

Popular in Questions Top

vegabook
I’m brand new to Phoenix and I have stripped one of the demo applications to the bone. I just want to get an svg up on the screen. Here i...
New
hariharasudhan94
Lets say I have map like this fetching from my database %{"_id" => #BSON.ObjectId<58eb1a7a9ad169198c3dXXXX>, "email" => ...
New
skosch
To my knowledge, put_in, Map.update etc. all have the one limitation of not automatically creating intermediate keys when needed (for exa...
New
stefanchrobot
What’s the safe way to decode a JSON string into a struct? I want to avoid calling String.to_atom. Jason.decode can give me a map with st...
New
greenz1
I have a phoenix application from which a user can download multiple(5-6) files of size 1MB. I couldn’t find anything related to sending ...
New
komlanvi
Hi everyone, I was playing with phoenix liveView but I run into an issue. I have a form and want to validate each input text when the te...
New
svb
Hi! Currently I want to submit a form by pressing the Enter key. However, since my input field is of type “textarea” this is just adds a...
New

Other popular topics Top

JeremM34
Hello, how can I check the Phoenix version ? Thanks !
New
minhajuddin
I have seen a lot of code which picks the first element from a list using Enum.at(0) instead of List.first. Is there a reason why people ...
New
baxterw3b
Hi guys, i’m new in the Elixir world, and i have to say, that i love it! i’m having some problem to understand anonymous functions with ...
New
aadeshere1
I have a another noob question about loop. Since elixir is immutable, while loop is not directly possible. total = 10 while total != 0 ...
New
gshaw
What is the idiomatic way of matching for not nil in Elixir? E.g., First way: defp halt_if_not_signed_in(conn, signed_in_account) when...
New
bsollish-terakeet
Credo is smart enough to check for (something like) this: assert length(the_list) == 0 with this response: Checking if an enum is empt...
New

We're in Beta

About us Mission Statement