Allow shadowing of function arguments only for val → var

There are occasions that I want to modify the value of a function argument within the method. I could create a var in the function with a similar name (called the argument x and the var x_) but this is begging for an accident to happen. It’s safer to call the argument x_ and the var x (but then the annotated name appears in the public interface and may be shown by the IDE):

fun foo(val binarySubtreeSIze_: Int) {
    var binarySubtreeSIze = binarySubtreeSIze_
    while (binarySubtreeSIze * 2 < BUF_SIZE) binarySubtreeSIze *= 2
    return binarySubtreeSIze
}

If shadowing of the same name was allowed only for val to var then the _ could be dropped.

fun bar(x: Int) { var x = x; // do something mutating with x }

There are arguments against shadowing argument values in scopes enclosed within the function but if this was only allowed at the outermost scope of the function then the creation of the shadowing var completely hides the original val and there’s no way to access the val and so no possibility of a mix up. All it would do is explicitly flip the val to a var.

I have a few real-world cases. Here’s a function that has two instances in the same function. This occurs more often than you would think. In this case below, the quality of producing output that is “minimal” is not possible (or meaningful) in certain scenarios, so when those are detected, the ‘minimal’ argument is set to false because it would cause an infinite loop otherwise. Similarly, the symmetry modes include Vertical, Horizontal, Diagonal, Rotational, etc, as well as one called Random. If the argument is Random, I replace it with a randomly chosen one.

fun generate(symmetry_: Symmetry, minimal_: Boolean = true): Grid {
    var minimal = minimal_           // val to var

    // for these narrow boxes, no minimal solution will exist in some symmetry modes
    if (prototype.boxWidth() == 1 || prototype.boxHeight() == 1)
        minimal = false

    var symmetry = derandomise(symmetry_)

    while (true) {
        val out = generatePotentiallyNonMinimal(symmetry)
        if (!minimal || Solver.isMinimal(out)) return out
    }
}

I agree with you, but luckily name shadowing is just a warning, not an error. In the few cases I use it I just use @Suppress("NAME_SHADOWING").
Also I still don’t like to use a var. You can simplify your example to

@Suppress("NAME_SHADOWING")
val minimal = if(yourCondition) false else minimal

Another option to solve this is to create a private function which does not do any of the argument checks and a public function which passes the corrected arguments to it.

Thanks, @Wasabi375. Nice suggestion :slight_smile: The symmetry argument could also be handled the same way as minimal of using a val with shadowing and suppresses the warning, but for me the @Suppress(“NAME_SHADOWING”) is a bit ugly and distracting. It is less conspicuous to use the _, though I don’t really like that either.

It would be good if this warning was suppressed by itself in the case of val → var in the outermost scope.

Can you think of any use cases where this could cause a problem? Because it hides the val completely and there wouldn’t be a wider scope where an inner value could “revert,” I can’t see any risks.

If you have a bigger function you might at a later point create a new variable and shadowing the argument without noticing it.

I’ve done that elsewhere, though I ended up in the same naming pickle! :cucumber: Should I call the public function generate() and the args-checked one generate_()?

Sometimes I just want to do something arithmetic where you don’t have a choice:

fun testCollatzConjecture(n_: Int, max: Int): Boolean { var n = n_ repeat(max) { n = if (n % 2 == 0) n / 2 else n * 3 + 1 if (n == 1) return true; } return false }

Apologies, I didn’t read your reply before hitting send!

In the base of a big function and shadowing and not noticing, it doesn’t have any impact because—if this is only allowed at the outermost scope—then the new value completely hides the old one. It’s the partial hiding in smaller inner scopes that is the problem (because the scope ends and the original value is visible again)

No this isn’t true in fact it’s really dangerous to do that. It opens you up to crash when writing asynchronous code as well as opens you up to side-effects. Best practice is to make a copy then return a new variable in the return statement and reassign whatever parameter you need to reassign.

Could you expand a little how, in the examples given, copying an argument into a local var could potentially cause a crash or a side-effect? In asynchronous code, arguments are already thread-local.

Here’s a real-world example (counting the Hamming Weight popcount) where I need to modify the argument or create a shadow var (for the val).

This is an algorithm designed to be super highly efficient so the cost of the creation of a duplicate variable is a real cost. (Yes, I know that java.lang.Integer.bitCount() wraps the popcount x86 machine instruction.)
`
fun popcount(_n: Int): Int {
// each bit in n is a one-bit integer that indicates
// how many bits are set in that bit
var n = _n

n = ((n and 0xAAAAAAAA) ushr 1) + (n and 0x55555555)
// Now every two bits are a two bit integer that
// indicate how many bits were set in those two
// bits in the original number

n = ((n and 0xCCCCCCCC) ushr 2) + (n and 0x33333333)
// Now we're at 4 bits

n = ((n and 0xF0F0F0F0) ushr 4) + (n and 0x0F0F0F0F)
// 8 bits

n = ((n and 0xFF00FF00) ushr 8) + (n and 0x00FF00FF)
// 16 bits

n = ((n and 0xFFFF0000) ushr 16) + (n and 0x0000FFFF)
// kaboom - 32 bits

return n

}
`