Local accumulators for cleaner comprehensions

The idea of a “special assignment operator” is already a rejected alternative in the proposal.

Similarly the idea for additional keywords for special assignment would similarly not pass for the same reasons.

2 Likes

It is fine to question the legitimacy, as long as it is accurate and constructive. Calling it a toy problem is neither. You could instead have asked: “can you provide more context about the problem statement?”. Also, it is not hard for us to try to find where we can apply this to.

I grepped Livebook and I found this:

{sections, {_branching_ids, warnings}} =
  notebook.sections
  |> Enum.with_index()
  |> Enum.map_reduce({MapSet.new(), []}, fn {section, section_idx},
                                            {branching_ids, warnings} ->
    # Set parent_id based on the persisted branch_parent_index if present
    case section.parent_id do
      nil ->
        {section, {branching_ids, warnings}}

      {:idx, parent_idx} ->
        parent = Enum.at(notebook.sections, parent_idx)

        cond do
          section_idx <= parent_idx ->
            {%{section | parent_id: nil},
              {
                branching_ids,
                [
                  ~s{ignoring the parent section of "#{section.name}", because it comes later in the notebook}
                  | warnings
                ]
              }}

          !is_nil(parent) && MapSet.member?(branching_ids, parent.id) ->
            {%{section | parent_id: nil},
              {
                branching_ids,
                [
                  ~s{ignoring the parent section of "#{section.name}", because it is itself a branching section}
                  | warnings
                ]
              }}

          true ->
            {%{section | parent_id: parent.id},
              {MapSet.put(branching_ids, section.id), warnings}}
        end
    end
  end)

and with this proposal that could be rewritten to:


  @@warnings = []
  @@branching_ids = MapSet.new()

  for {section, section_idx} <- Enum.with_index(notebook.sections) do
    case section.parent_id do
      nil ->
        section

      {:idx, parent_idx} ->
        parent = Enum.at(notebook.sections, parent_idx)

        cond do
          section_idx <= parent_idx ->
            @@warnings = [
              ~s{ignoring the parent section of "#{section.name}", because it comes later in the notebook}
              | @@warnings
            ]
    
            %{section | parent_id: nil}
    
          !is_nil(parent) and MapSet.member?(@@branching_ids, parent.id) ->
            @@warnings = [
              ~s{ignoring the parent section of "#{section.name}", because it is itself a branching section}
              | @@warnings
            ]
    
            %{section | parent_id: nil}
    
          true ->
            @@branching_ids = MapSet.put(@@branching_ids, section.id)
            %{section | parent_id: parent.id}
        end
    end
  end)

Is it better? Worse? You decide.

So if we want to question the problem statement, let’s put the work into it. :slight_smile:

16 Likes

Both are confusing as hell. :003: I would immediately break this code in smaller chunks.

Let’s write it off as cultural differences then? I am here for quite a long time, I’d think you would know that I don’t argue in bad faith. I engage because I care.

Here we are in full agreement. I’ve copied the code from the archived repo and will attempt to devise a better idiomatic Elixir that could make the case for enriched for (or other constructs). Might have a crack at the Livebook snippet as well (will look if that code in particular is covered with tests so as to be as close to the original problem as possible, too).

The tricky bit is that I am actively onboarding in a new job so I have to prioritize that. And I am afraid I’ll miss the train on this proposal but oh well.

4 Likes

Agreed, they both could be better. I went for the trivial conversion.

Definitely. That was never into question. :heart: I just don’t want problems to be dismissed for the wrong reasons.

5 Likes

That’s very much acceptable I think

I don’t have many restaurants invested in either side of this discussion, but I don’t understand the strong opposition.

Many of the people against it seem to be very «I wouldn’t need this» in how they argue and I think our lens should be more neutral.

Jose didn’t say (I think?) why he wanted to come up with this or what inspired him, but I would put my two cents on it coming from the livebook side of things.

If you work with system or web dev you do the occasional Enum spaghetti wrangling which is fine. If you work in Livebook all day as a data scientist you do this all day all week.

I think Elixir as a language would have a lot of benefit in making the lives of these people simpler. I haven’t done a survey to put it like that, but purity in functional style is probably not the main allure of Livebook.

I can certainly see this making data wrangling scripts and such becoming both quicker to writer and easier to read at a glance with this addition.

And I say that as someone who only really sometimes miss early returns (which can always be solved by breaking your logic up in smaller pieces and into a with chain) after about ten years with Elixir as my daily professional driver.

Someone wrote that Elixir should be striving for less syntax. I strongly disagree. Elixirs core selling point has always been to stand on Erlangs shoulders. And those shoulders has been defined by a single word: pragmatism.

This change caters to pragmatism through and through.

10 Likes

I can surely see several places in some of my codebases where a construct like this would have saved me from reducer-hell.

As most of you I hate the @@, which is why I think it is a good solution.

I have seen the proposal of introducing a keyword mut or likewise and then use a generic variable name.
This would make it impossible to see if a variable is a mutable one easily further in your code.

I have also read the comment by @D4no0 and the example

@@magic_value = 20
Enum.map([1, 2 ,3], fn el -> if el == :rand.uniform(3), do: @@magic_value = 21 end)

Now reading @josevalim’s description I know the above code will not change @@magic_value as it is in another scope, a new function, but it is confusing as hell.

Therefore, don’t shoot me, I would prefer the declaration of said variables to be explicit, but still keep the @@ prefix

mut @@magic_value = 20

This would enable the compiler to catch trying to assign a value to a non-declared variable inside said scope.

2 Likes

When I was reading this proposal I had mix feelings :laughing:

While sometimes I miss the ability to change a value in the middle of a map, I wouldn’t have thought to add it to the language. However, this solution offers scoped local accumulators, which is an interesting approach!

Currently, the flow is clear: you want to change two things(or more) at the same time? You have to pass both to the Enum.map. While this requires some data gymnastics, I’ve gotten used to it over time.

Something to consider is if local accumulators are introduced, it might split the community into two groups: those who use local accumulators and those who stick to ‘pure’ variables.

That said, If the intention is to help newcomers get on board faster, then it’s a decent solution.

It’s fantastic that we can implement this today without significant changes to the language. However, we all need some time to think about it and adjust our feelings and think about the consequences. From now on, whenever I write a function, I’ll be thinking about how I might have done things differently if I had local accumulators at my disposal.

3 Likes

I read somewhere that they made the keyword mutable and not mut in F# to give you extra time to think about whether you really want to be using mutable variables :slight_smile:. I checked my [realtively small] F# codebase and there are three usages of mutable. Two are because you can’t shadow variables like in Elixir and the other was in a 6 line function. I think if this change happened, most people would continue to code in a functional manner.

Personally, I would prefer no @@ and just to let the editor display the “mutable” value differently. And as much as it bothers me writing x = if something do ... else x end I’m on the fence about if something do @@x = 1 end.

I too think that some of us are getting a little too excited about the apocalypse that this would bring. It would be nice to have a couple of forks of Elixir with the options so we can have a play with it and see how it feels.

The community is already split on people who tag tuples in with and match on them in else and those that use case, those that add functions to locals_without_parens and those who use brackets, those who use the formatter and those who don’t. Somehow we survive :slight_smile:

4 Likes

In this case you would get a compilation error!

Why? Because you are shadowing the @@magic_value?

Gave it another thought and I generally like this proposal.

  • First, as mentioned earlier, I see a good use for it in situations other than just comprehensions, e.g. in cond’s:
  cond do
    !first_thing_checks? ->
      :bad

    ( @@foo = expensive_call( args)) && !@foo.this_ok? ->
      :bad

    @@foo && @@foo.surely_not_this? && something_bad ->
      :bad

    true ->
      :good
  end
  • Second, as another user here noted, I too find @@ to be an obscene token, but it’s ok it’s obscene because it’s very noticeable meaning any grouping of/littering with those (i.e. an abuse) will be easy to spot. Also, the reason why I prefer it over using plain variable names e.g. mut foo.
1 Like

After I overslept with the problem, I decided to comment on it again.

Here is the pure comprehension solution, verbose

for {map, map_idx} <- Enum.with_index(input, 1), reduce: {1, []} do
  {idx, result} ->
    idx = if map["reset_lesson_position"], do: 1, else: idx

    {idx, lessons} =
      for lesson <- map["lessons"], reduce: {idx, []} do
        {idx, acc} -> {idx + 1, [Map.put(lesson, "position", idx) | acc]}
      end
      
    map = map |> Map.put("position", map_idx) |> Map.put("lessons", Enum.reverse(lessons))
    {idx, result ++ [map]} # to avoid `Enum.reverse/1`
end

This is not easily generalizable (unlike iteraptor’s solution I mentioned above,) but it solves the exact problem stated. It’s readable by anyone familiar with Elixir and it is arguably more succinct compared to map_reduce/3, although it uses the same approach.

Would I personally write this in the production code? Nope. I’d do it with (:drum: :drum: :drum:) Enum.chunk_by/2.

iex(1)> Enum.chunk_by(input, & &1["reset_lesson_position"])
[
  [
    %{
      "lessons" => [%{"name" => "Welcome"}, %{"name" => "Installation"}],
      "reset_lesson_position" => false,
      "title" => "Getting started"
    },
    %{
      "lessons" => [
        %{"name" => "Addition / Subtraction"},
        %{"name" => "Multiplication / Division"}
      ],
      "reset_lesson_position" => false,
      "title" => "Basic operator"
    }
  ],
  [
    %{
      "lessons" => [%{"name" => "Mutability"}, %{"name" => "Immutability"}],
      "reset_lesson_position" => true,
      "title" => "Advanced topics"
    }
  ]

Then it’s a matter of Enum.with_index/2.


I am positive (this is a not humble opinion :slight_smile:) that every case should be attempted in the way suggested by its shape. Semi-mutability which is not a mutability but rather a syntactic sugar, is not a silver bullet. This particular example, as shown above, might be solved more elegantly. Another example might ring a bell, but this one does not.

As an author of some libraries, I sometimes have a feeling that hey, I should add something cute to it, and I do, and I always regret it. I could not respect more your vision of what is good for the language/ecosystem, and what’s not, @josevalim, but I think you are falling into a trap here. Local semi-mutability which is not a mutability but rather a syntactic sugar cannot even be named properly without confusing readers.

It’s not something that would help newcomers (IMHO) because of “ok, there is some mutability, they say, let me try with what I am familiar to” in many cases when the pure functional solution fits better.

It’s not something that would impress gray-bearded alchemists, because we know how to do it better without breaking the paradigm.

The question “who is the main audience” is widely open. It’s not an ad-hoc, the proposed solution with double-at already is a backward-incompatible change.

That all said, I am fine with whatever José thinks about it, because in my experience he is usually right and I am not (that’s the 102% fact.) But at the moment my personal opinion is we need more examples to convince the audience this is a good addition that brings more value than it blurs the boundaries of the paradigm we all are accustomed to.

11 Likes

Got it.

Question, what would happen if someone wrote

def somefn do
   ... no mention or assignment of @@var
  if sometest, do: @@var = true
  ...
  if @@var do
  ...
 end
end

I assume a compilation errorr, where would the compiler fail?

I just tried to refactor this a bit and came up with this:

defmodule Result do
  use TypedStruct

  typedstruct do
    field(:branching_ids, MapSet.t())
    field(:warnings, list(String.t()))
  end

  def new(), do: %__MODULE__{branching_ids: MapSet.new(), warnings: []}

  def add_ignored_parernt_warning(self, name),
    do: %{
      self
      | warnings: [
          ~s{ignoring the parent section of "#{name}", because it comes later in the notebook}
          | self.warnings
        ]
    }

  def add_branching_warning(self, name),
    do: %{
      self
      | warnings: [
          ~s{ignoring the parent section of "#{name}", because it is itself a branching section}
          | self.warnings
        ]
    }

  def track_branches(self, id),
    do: %{self | branching_ids: MapSet.put(self.branching_ids, id)}

  def branching_member?(self, id), do: MapSet.member?(self.branching_ids, id)
end

defmodule Example do
  def do_clean(notebook) do
    {sections, result} =
      notebook.sections
      |> Enum.with_index()
      |> Enum.map_reduce(Result.new(), fn {section, section_idx}, result ->
        case section.parent_id do
          nil ->
            {section, result}

          {:idx, parent_idx} ->
            parent = Enum.at(notebook.sections, parent_idx)

            cond do
              section_idx <= parent_idx ->
                {%{section | parent_id: nil},
                 Result.add_ignored_parernt_warning(
                   result,
                   section.name
                 )}

              !is_nil(parent) && Result.branching_member?(result, parent.id) ->
                {%{section | parent_id: nil},
                 Result.add_branching_warning(
                   result,
                   section.name
                 )}

              true ->
                {%{section | parent_id: parent.id}, Result.track_branches(result, section.id)}
            end
        end
      end)

    {sections, result.warnings}
  end
end

I’m not sure it’s better or worse. But it’s different. I was time constrained and I have some ideas on how to improve the refactoring some more, but it can already give an example of possible options to change the code.

BTW I saw that F# also has mutable variables, wondering if we can survey the F# users(and the creators) to see how do they use it and what do they like or don’t about it

6 Likes

Also consider

def somefn do
  # ... no mention or assignment of @@var
  for element <- [1, 2, 3] do
    @@var + 1
    element
  end
end

This is why I think the @@ syntax would require a different initialization declaration syntax (ex @@var 0) from update (ex @@var = @@var + 1).

1 Like

I like the idea of the accum block; in fact, I had thought about it myself, as did a few others in this thread. It just feels like a natural fit for Elixir. To me, it serves as a signal for the scope, somewhat discouraging its usage (which is good) due to the longer syntax and indicating that it’s something special.

I also believe that a visual marker is necessary to differentiate it from “normal” variables. Perhaps something like @@my_var, !my_var, &my_var, &&my_var, etc. The last ones might give off a “mutable” vibe for those familiar with C/C++/Rust.

I don’t think there needs to be a separate operator for this, considering how it would work with pattern matching.

So, for me, something like this looks good:

accum &my_var: 123 do
  ...
end
3 Likes

This is taken by anonymous function captures.

(&IO.inspect/2).(:foo, label: "bar")
bar: :foo
#⇒ :foo
1 Like

Yeah, but the function capture requires arity, so I think it would be possible to distinguish it by AST:

iex> quote do &my_fun/2 end
{:&, [],
 [{:/, [context: Elixir, imports: [{2, Kernel}]], [{:my_fun, [], Elixir}, 2]}]}
iex> quote do &my_var end
{:&, [], [{:my_var, [], Elixir}]}

The problem is that both “&” and “&&” are already Elixir operators, so is it even possible?

1 Like