<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style>
</head>
<body lang="EN-US" link="blue" vlink="#954F72" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal">Hi Tim, Levente,</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I considered shadowing a feature until I noticed it’s being frowned upon.</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">> But, since I like the feature, a better option would be to adjust
<br>
the highlighting of such variables to make shadowing more obvious.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">+1 </p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I may have missed some discussion about this; isn’t it a similar situation as when you intentionally use an undefined variable and get a warning when accepting the method? You get a warning a that’s it. </p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">In case of Process tests it’s an oversight on my part, apologies; I’m enclosing fixed versions of the tests:</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">ProcessTest>>testResumeTerminatingProcess (semaphore is shadowed)<br>
ProcessTest>>testTerminateTerminatingProcess (semaphore is shadowed)<br>
<br>
</p>
<p class="MsoNormal">And yes, cases like this create an unnecessary confusion whether shadowing is intentional or an oversight, so at least a comment stating the reason for using a shadowed variable would be helpful, I guess.</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Best,</p>
<p class="MsoNormal">Jaromir</p>
<p class="MsoNormal"><span style="color:#8FAADC"><o:p> </o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="mso-element:para-border-div;border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="border:none;padding:0in"><b>From: </b><a href="mailto:leves@caesar.elte.hu">Levente Uzonyi</a><br>
<b>Sent: </b>Sunday, January 1, 2023 1:20<br>
<b>To: </b><a href="mailto:squeak-dev@lists.squeakfoundation.org">The general-purpose Squeak developers list</a><br>
<b>Subject: </b>Re: [squeak-dev] A lot of shadowed variable names in Squeak 6.0 needing clearing up</p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt">Hi Tim,<br>
<br>
I have got no problem with shadowed variables. It's a feature to be able <br>
to shadow those variables.<br>
If you want to get rid of them, then you should consider making the <br>
compiler raise an error instead of spamming the transcript with <br>
warning-looking messages. If you don't do so, they will creep back despite <br>
the current wave of cleanup.<br>
But, since I like the feature, a better option would be to adjust <br>
the highlighting of such variables to make shadowing more obvious.<br>
<br>
On Sat, 31 Dec 2022, tim Rowledge wrote:<br>
<br>
> An excellent example is Random>>#nextBytes:into:startingAt:<br>
><br>
> 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.<br>
<br>
In Random, shadowing is pretty much intentional.<br>
<br>
The index instance variable is only accessed by private methods <br>
implementing the PRNG logic itself.<br>
If you think the name is too generic, then you may want to rename it to <br>
nextStateIndex, as it holds to the index of the next state to generate <br>
the next random values.<br>
>From the Random object's point of view, the name index is okay IMO.<br>
<br>
In #nextBytes:into:startingAt:, index is obviously the running index of <br>
the ByteArray to be filled up with random values. So, it clearly should <br>
not touch the instance variable. If you don't like the name index, I <br>
suggest using currentIndex instead.<br>
<br>
For the other methods of Random, I do not fancy calling variables "i".<br>
I understand why it may be tempting to use single letters or <br>
abbreviations, especially without code completion, but I prefer <br>
descriptive names.<br>
Also, RIP my beautiful methods :(. :)<br>
<br>
<br>
Levente<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>