Fix scope warning without adding extra code

Code with the warning

foo = 3
condition = false

# update foo only if condition is met
if condition do
  foo = foo + 1
end

IO.puts foo

foo = 3
condition = false

Code without warning, but it needs a superficial else clause

# update foo only if condition is met
foo = if condition do
  foo + 1
else
  foo
end

IO.puts foo

How can the behavior be preserved without the superficial else clause?

1 Like

You could rewrite it as

foo = (condition && foo+1) || foo

or even

foo = condition && foo+1 || foo

but most people would not find this easier to read.

It isn’t superficial. It makes it clear that an expression is returning one value or another. You can read more about the reasoning here: http://elixir-lang.org/blog/2016/06/21/elixir-v1-3-0-released/

It is kind of superficial, right? Because in the earlier version we didn’t need an else clause because the value of foo wasn’t being changed. I am not sure if there is a way to conditionally reassign the data to a variable.

The code is terse but the semantic structure is the same. You are right about the readability. I’d rather use a more readable approach.

The reason this warning was introduced, is to be able to enforce this in future releases.

The advantage of this is that scopes will then work exactly the same, everywhere in Elixir. Until now, if/else were the only constructs that broke scoping (allowing you to set things inside them that were accessible outside afterwards).

This change will allow the compiler to do some fancy rewriting that might increase performance. It also allows you to easier move functionality to separate functions, as you don’t have to worry about breaking anything when doing that.

As for the if-statements where we either want to modify a value very slightly or keep it the same, maybe some kind of syntactic sugar could be added for them.

The semantic structure is very different. The reassignment version as an if expression that discards its return value. Despite discarding the return value, we find that it implicitly returned any number of values via reassignment that affect later computation.

The version now promoted by 1.3 has semantics consistent expression oriented and functional programming wherein values you want to use later must be explicitly returned.

1 Like

I meant that comment for Fix scope warning without adding extra code

I understand the need for consistent scoping, maybe what we need is some syntactic sugar like @Qqwy said :slight_smile:

Some syntactic sugar might be a good idea to make this code DRY

This was discussed at length on the core mailing list, I’ll see if I can find a link.

I don’t understand what DRY has to do with it. In the case where you have just one if else that you’re using to rebind to an outer value, there isn’t any repetition. There’s explicitness, but no part of that is a repeat of some earlier part.

If you have many such if else things in a row there are trivial function patterns that will obviate the need for a multitude of if else blocks without any special syntax.

2 Likes

It’s also wise to remember that DRY is a guideline not a unbreakable rule. It’s better not to repeat yourself but sometimes it’s unavoidable and contorting the code to avoid repeating something isn’t a great answer either. The first aim of any code that any of us write should be to be easily understood by other developers. Any other goal is secondary to that.

Syntactic sugar (like the food additive) should be used sparingly and carefully. Changing the value of a variable if some condition is met isn’t a terribly functional approach anyway. Rather than modifying “foo” you should be assigning the result to a different variable that represents what the new value means. For (a contrived) example:

sales_with_tax = if (state == :Michigan) do
   sale_amount * 1.06
else
   sale_amount
end

If you’re simply incrementing foo if a condition is met then I think you’re missing a more fundamental property of FP anyway. Of course I don’t know your use case so I’m simply speculating but if you effectively need to mutate a value that’s a code smell in FP.

1 Like

The code in question is on Github at: https://github.com/rawcodehq/webmonitor/blob/master/lib/webmonitor/uptime_calculator.ex#L67 . I am updating the value of downtime on https://github.com/rawcodehq/webmonitor/blob/master/lib/webmonitor/uptime_calculator.ex#L74

One thing we are not acknowledging is that we’d have an extra branch statement, and more code usually equates to a higher probability of bugs. I think we can all agree that

for (i = 0; i < arr.length; i++) {
  work(arr[i]) 
}

has more scope for bugs than

Enum.each(arr, &work/1)

Because there is only so much you can mess up.

I understand that the language imposes the scope restrictions, I may be missing something basic here.

It does not feel like you’re taking seriously the responses offered here. I’ve spent some time at this point linking the official blog post, articulating semantic differences, explaining the explicit vs implicit trade off, and none of it has been addressed.

Read the blog post I linked originally, it explains at length the decision making process here. If you have particular remarks about that reasoning, please quote the part you take issue with and explain why.

2 Likes

So, ok, now I’m confused.

Are you asking why you get the warning? If so, then @benwilson512 has already linked an excellent resource on the subject of why the change was made.

Are you asking how to restructure your code so the warning goes away? If so, maybe you can tell us what’s unclear from the responses which have already been posted.

Are you asking for a way to modify the code so that you don’t need the else branch? I don’t think there is a way to modify the code such that you don’t need the else branch. The thing is you need to think of if as an expression not a statement. An expression always returns a value hence you need to define what value should be returned when the expression evaluates to true and when it evaluates to false.

It’s also wise to bear in mind that right now this is a warning; in future, it’s likely to become a compiler error and you should address the issue in your code now.

Yes, I understand why the warning is emitted and the scoping rules. I am just trying to find a way to avoid the else loop if there is a way to do that.

I apologize if I came off as not taking the suggestions here seriously. I do appreciate your help. I’ll try and find the mailing list discussion if I missed anything.

What’s coming in Elixir 1.3 by Daniel Perez touches on this issue:

However, in 1.2, we could write the following,

def imperative_duplicate(str, n, opts \\ []) do
  if opts[:double] do
    n = n * 2
  end
  String.duplicate(str, n)
end

which is actually quite common.

This will be deprecated from 1.3. This is a little more constraining, and does not have a solution that work for every cases, as the previous warning did. For the above code, we could for example, add a helper as follow:

def imperative_duplicate(str, n, opts \\ []) do
  n = double_if_true(n, opts[:double])
  String.duplicate(str, n)
end
defp double_if_true(n, true), do: n * 2
defp double_if_true(n, false), do: n

or simply rewrite like this:

def imperative_duplicate(str, n, opts \\ []) do
  n = if opts[:double], do: n * 2, else: n
  String.duplicate(str, n)
end

Both are a little more verbose, so I would like to discuss why this change has been introduced, and why I think it is a good thing.

Daniel then goes on to explain why this change was required. I understand the issue but was just trying to see if there is any simpler solution.

1 Like

These are the simplest options available.

2 Likes
Code with the warning
> foo = 3
> condition = false

> # update foo only if condition is met
> if condition do
>   foo = foo + 1
> end

> IO.puts foo

> foo = 3
> condition = false

For whatever it’s worth, it’s wise to also remember in the code above there is an implied else. The implied else would be to simply return foo unaltered.

While having to add the else may seem like an imposition it could also be argued that it makes the code more explicit and explicit is usually better than implicit especially when it comes to code.

Again, for whatever it’s worth.

1 Like