Retrieving data from list of maps based on field values derived from another list of maps. Any code smells?

Will love to hear your critique and comments

I have two lists of maps, location_list and user_location_list based off two structs: Location and UserLocation respectively.

Here, each list shows a partial listing of all the fields for brevity.

location_list = [%Location{location_id: 1, display_name: "Location One"}, %Location{location_id: 2, display_name: "Location Two"}]

user_location_list = [%UserLocation{location_id: 1, user_id: 1}, %UserLocation{location_id: 2, user_id:1}]

Basically after retrieving the location_id values from user_location_list based on a user_id search, I then want to get the corresponding location records from location_list with the fields location_id and display_name (so I can populate a list in Phoenix EEx template with relevant hyperlinks)

My code is as follows:

location = Enum.map(location_list, fn location -> location = Map.take(location, [:location_id, :display_name]) end)

This is to strip out the Struct so it can read [%{location_id: 1, display_name: "Location One"}...] AND display only a subset of the fields in the Location struct

user_location = Enum.map(user_location_list, fn user_location -> user_location.location_id end)

So this produces a list [1, 2]

#this will contain the filtered locations based on values in user_location
dropdown_list = Enum.filter(location, fn (location) -> location.location_id in user_location end)
  1. Please critique my approach so I can get better.

  2. I really hope I can construct a pipeline here but I dont know how to. For starters I would be thinking like

dropdown_list = location_list 
|> Enum.map(location_list, fn location -> location = Map.take(location, [:location_id, :display_name]) end)
|> Enum.filter(&l, fn (location) -> location.location_id in user_location end)

But it does not seem to work.

  1. And I have heard so much about Enum.reduce. Is there any way it can be applied here to make the code better?

  2. Finally, any suggestions how you would have named the variables location and user_location (as they really are lists after all but location_list is also a list itself)

Thanks very much.

If I understood you correctly this is what you want:

defmodule Location do
  defstruct [:display_name, :location_id]
end

defmodule UserLocation do
  defstruct [:location_id, :user_id]
end

defmodule Example do
  def sample(locations, user_locations) do
    location_filter_ids = user_locations |> Enum.map(& &1.location_id) |> Enum.uniq()

    Enum.reduce(locations, [], fn location, acc ->
      if location.location_id in location_filter_ids do
        [Map.take(location, [:location_id, :display_name]) | acc]
      else
        acc
      end
    end)
  end
end

locations = [
  %Location{location_id: 1, display_name: "Location One"},
  %Location{location_id: 2, display_name: "Location Two"}
]

user_locations = [
  %UserLocation{location_id: 1, user_id: 1},
  %UserLocation{location_id: 2, user_id: 1}
]

Example.sample(locations, user_locations)

# result
[
  %{display_name: "Location Two", location_id: 2},
  %{display_name: "Location One", location_id: 1}
]

With this code instead of map + filter you have only one loop and map part is applied only to filtered results.

2 Likes

Thank you very much Eiji. Im beginning to grasp the capture operator as well as the other functions. :smile:

So the main idea of all these operations such as map and reduce is to compress all the operations in a single iteration?

Well reduce as well as head-tail function pattern-matching have many use cases and doing all operations in a single loop (my example) is definitely one of them.