Re: Use readonly wherever possible?

[Alan D. Salewski]

On 2020-05-06 10:44:08, D630 spake thus:
> Recently, I had to review one of my team members shell code. She is a Java
> developer, and used to declare variables with the final keyword. So in shell
> she was constanly using the readonly attribute like:
> 
> foo() {
>       local -r bar=$1
>       local -r lich=$2
>       echo "$((bar * lich))"
> }
> 
> I think that's unusual code in the shell-code universum. My first reflex was
> to say "No, don't do that", but I hesitate with my answer, because I have no
> idea what happens exactly in bash when a variable is declared readonly. So
> is there any advantage in using readonly wherever possible?

Wherever possible? I would say, "no".

In above example, the readonly attribute is mostly innocuous but also does not
buy much. For a larger function, it might help catch accidental/unintended
assignments (within the local function) during testing, but it is just as
likely to /introduce/ bugs because (generally speaking) the downstream effects
of readonly variables are not as well understood as they might be, and
developers copy/paste code they do not do fully understand.

In the above example, the intent of the developer seems to be to prevent
accidental/unintended assignment of 'bar' and 'lich' further down within the
function. And that's fine, as far as it goes, as the function does not call
other functions; this is the aspect in which the usage is "mostly innocuous".

However, the readonly attribute communicates an intent and usage of the
variable for both the current scope and downstream scopes, and I would guess
that is probably not what was intended by the programmer who wrote the above
function. For the following slight elaboration, would the developer expect
baz() to produce a fatal error? I'm just speculating, but my guess is that it
would be a surprise.

    baz() {
        bar='BAZ'             # fatal err: "readonly variable"
        echo "in baz: $bar"   # code never gets here
    }

    bar() {
        local -r bar='BAR'    # okay; shadows $bar from foo()
        echo "in bar: $bar"   # prints 'BAR'
        baz                   # never returns
    }

    foo() {
        local -r bar=$1
        local -r lich=$2
        echo "in foo: $((bar * lich))"  # prints 15
        bar                             # never returns
    }

    foo 3 5


When other developers see the readonly attribute, their Spider Sense will
start tingling (as yours did). Either something intentional is going on that
they need to pay special attention to (extra reasoning about the scope), or
(just as likely) the feature is being used incorrectly. Either way, it slows
down the reading of the code to contemplate what the situation is; it /may/ be
correct, but it is probably not obviously correct[0].

Unless the non-lexical scope impact is what is specifically intended, leave
off the readonly attribute.

Some related discussion about readonly behavior from bug-bash:

    https://lists.gnu.org/archive/html/bug-bash/2011-02/msg00194.html
    https://lists.gnu.org/archive/html/bug-bash/2012-04/msg00088.html
    https://lists.gnu.org/archive/html/bug-bash/2012-04/msg00099.html

Take care,
-Al

[0] https://wiki.c2.com/?TwoWaysToDesign

-- 
-----------------------------------------------------------------
a l a n   d.   s a l e w s k i                   address@hidden
1024D/FA2C3588 EDFA 195F EDF1 0933 1002  6396 7C92 5CB3 FA2C 3588
-----------------------------------------------------------------