> On 2022-12-31, at 4:20 PM, Levente Uzonyi <leves at caesar.elte.hu> wrote:
> 
> Hi Tim,
> 
> I have got no problem with shadowed variables. It's a feature to be able to shadow those variables.

Fascinating - I don't think I've ever heard that suggestion before. I would argue that it acts to confuse by overloading a name, and code is quite confusing enough! What is it that you like about it; is it simply the freedom to choose a name? 


> If you want to get rid of them, then you should consider making the compiler raise an error instead of spamming the transcript with warning-looking messages. If you don't do so, they will creep back despite the current wave of cleanup.

I agree, and I think ct offered such an exception a day or two ago?


> But, since I like the feature, a better option would be to adjust the highlighting of such variables to make shadowing more obvious.

Yes, but... highlighting can be helpful *if* you are in a context where you are presented with it. And if it actually works visually, which can be quite tricky. And if something hasn't turned off the highlighting.

> 
> On Sat, 31 Dec 2022, tim Rowledge wrote:
> 
>> An excellent example is Random>>#nextBytes:into:startingAt:
>> 
>> It refers to a temp named 'index' which shadows an instvar. Should the method be just using that instvar, or a different named temp? I *think* the latter but it's possible Levente would suggest otherwise. All I can be sure of is that the RandomTest class reports all ok.
> 
> In Random, shadowing is pretty much intentional.
> 
> The index instance variable is only accessed by private methods implementing the PRNG logic itself.
> If you think the name is too generic, then you may want to rename it to nextStateIndex, as it holds to the index of the next state to generate the next random values.
> From the Random object's point of view, the name index is okay IMO.
> 
> In #nextBytes:into:startingAt:, index is obviously the running index of the ByteArray to be filled up with random values. So, it clearly should not touch the instance variable. If you don't like the name index, I suggest using currentIndex instead.

Those are all good points and exactly why I wanted input from people that had some history with the methods I was touching. For minimum disturbance I'd probably go with 'currentIndex' for the loop var.

> 
> For the other methods of Random, I do not fancy calling variables "i".
> I understand why it may be tempting to use single letters or abbreviations, especially without code completion, but I prefer descriptive names.

Certainly descriptive names are A Good Thing. I would point out that Old Farts tend to think of  'i' as a descriptive name for an indexing variable, probably as a result of the brain damage engendered by having been taught FORTRAN as a larva.


> Also, RIP my beautiful methods :(. :)

Beauty is such an ephemeral thing...


tim
--
tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
Strange OpCodes: MW: Multiply Work