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
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
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/3which 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 ![]()
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. ![]()
adamu
Hopefully this is at least par for the course
. @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
Popular in Questions
Other popular topics
Categories:
Sub Categories:
Forums
Popular Tags
- #ecto
- #liveview
- #troubleshooting
- #learning-elixir
- #deployment
- #library
- #erlang
- #testing
- #genserver
- #mix
- #absinthe
- #remote-other
- #otp
- #plug
- #how-to-question
- #macros
- #postgres
- #channels
- #elixirconf
- #exunit
- #discussion
- #code-sync
- #javascript
- #podcasts
- #onsite
- #dialyzer
- #docker
- #authentication
- #umbrella
- #full-time-contract
- #podcasts-by-brainlid
- #ecto-query
- #elixir-ls
- #phoenix_html
- #iex
- #blog-post
- #graphql
- #genstage
- #ai
- #websockets
- #supervisor
- #elixirconf-us
- #advent-of-code
- #distillery
- #processes
- #forms
- #api
- #metaprogramming
- #hex
- #security









