Updating a list that is passed from parent functions

def create(conn, params) do
    conn 
      |> render_json_response(bulk_convertor(params))
  end

  def bulk_convertor(%{"file" => %Plug.Upload{content_type: "text/csv", filename: basename, path: fullpath}}) do
    list = [~w(old, new)]
    fullpath
    |> read_csv(list)
    |> form_response
  end

  def read_csv(path, list) do
    if path |> File.stream! |> Enum.count <= 50 do
      path
        |> File.stream!
        |> Parser.parse_stream
        |> Stream.map(fn [arg] -> %{url: arg} end)
        |> iterate_urls(list)
    else
      %{error: %{message: "Limit Exceeded"}}
    end
  end

  def iterate_urls(mapped_stream, list) do
    mapped_stream
      |> Enum.each(fn(url) -> url_convertor(url, list) end)
    %{response: %{message: "Request Initiated"}}
  end

  def url_convertor(%{url: val}, list) do
    with {:ok, %Route{} = route} <- App.create_route(%{"url" => val}) do
      route
        |> App.redirect_url
        |> add_to_list(route, list)
    end
  end

  def add_to_list(new_url, %{url: original_url}, list) do
    list = List.insert_at(list, -1, ~w(#{original_url}, #{new_url}))
    list
  end

The list content is not updated whenever I try to print it somewhere using IO.inspect it shows the initial list. I am trying to update the content of the list based on the data parsed from the csv which i am parsing using the nimble_csv library of elixir.

If I try to print the list inside

add_to_list

only two rows are visible the header that i added initially and last row from the CSV.

Is there anything I am doing wrong here.

You should provide more info about what you are trying to achieve and where it breaks, so people can help you.

That said, the first thing that stands out to me is that you are using Enum.each, which should be used for side effects and does not modifies the collection in place (actually nothing really does modify collections in place in Elixir). Not sure if that’s the problem as I miss context, but that’s one place to start.

1 Like

The list content is not updated whenever I try to print it somewhere using IO.inspect it shows the initial list. I am trying to update the content of the list based on the data parsed from the csv which i am parsing using the nimble_csv library of elixir.

If I try to print the list inside

add_to_list

only two rows are visible the header that i added initially and last row from the CSV.

Is there anything I am doing wrong here.

Do you have anything alternative to Enum.each or the way I get directly get the contents of CSV as list.

Thanks.

Variables in elixir are immutable so you cannot change the value of a parameter from within a function like you are trying to do. Instead you need to return the updated value from the function. Here’s a blog post I found about the topic:

2 Likes

Hi @lucaong & @axelson,

Thank you for the update and now I am able to come up with the solution.
My issue got resolved after I changed from

Enum,each to Enum.flat_map and

List.insert_at(list, -1, ~w(#{original_url}, #{new_url})) to List.insert_at([], -1, ~w(#{original_url}, #{new_url}))

Now my url_convertor is returning list itself so no need to update/create new list.

Thanks.

2 Likes

This could be replaced by…

List.insert_at(list, -1, ~w(#{original_url}, #{new_url})) 

Also it’s not common to push at the end of the list like this… the common way is to push at the head, and reverse the list if required.

[~w(#{original_url}, #{new_url}) | list]

Doesn’t ~w(#{original_url}, #{new_url}) evaluate to [original_url <> ",", new_url]. Is that , at the end of the original_url really wanted?

Also, if you really want to add as last item, list ++ [~w[…]] works as well and is usually more concise. This version is also slightly more efficient, as it does not need to iterate and then count back again.

Anyway, usually we avoid appending to a list in a loop/recursion, as it will make the algorithm quadratic in its runtime. The longer the list is, the longer it takes to append to it, as it has to be copied fully.

Therefore we usually do as @axelson already suggested and build the list in “reverse” and reverse again after it is ready.

This will read the full file once, then you read it again if its small enough. Perhaps limit file size rather than item count?

1 Like

Hey @NobbZ,

I got rid of that additional , .
Can you please give example for build the list in “reverse” and reverse again after it is ready.
As I want to iterate through each row in the csv then add it to that list.

Also my add_to_list function looks like this now.

def add_to_list(new_url, %{url: original_url}) do
    List.insert_at([], -1, ~w(#{original_url} #{new_url}))
  end

Thanks.

In Elixir, lists are linked lists and, as every other data type, immutable. Therefore, it is very efficient to prepend elements at the beginning of them: the new list is just the prepended element and a pointer to the previous list, no copying necessary. Conversely, appending to the end involves copying over the whole list.

For this reason, if you have to append many elements to a list, it is usually better to build the list in inverse order by prepending the elements, and only reverse the whole list at the end. This reduces copying and traverses the whole list only once, for the reversing.

For an example, imagine you have a list of numbers, and you want to build a list of the same numbers, each multiplied by 2, in the same order. You would normally use Enum.map, but if we pretend for the sake of the example that Enum.map did not exist, you could do something like this:

def double_numbers(numbers) do
  double_numbers(numbers, [])
end

defp double_numbers([], doubled) do
  Enum.reverse(doubled)
end

defp double_numbers([first | rest], doubled) do
  double_numbers(rest, [first * 2 | doubled])
end

In the example, we are recursively building the result list in reverse by prepending one element at a time, and than, only at the end when the original list is fully consumed, reversing it.

3 Likes

I see you marked my last reply as the solution, happy that you found it helpful :slight_smile:

That said, one quick suggestion: only one reply in the thread can be marked as the solution for your question, so you should mark the reply that you feel is solving the original question of the thread. This way, your thread can help other people in the future that have similar questions.

1 Like