Re: [SCM] Debian package checker branch, master, updated. 2.5.2-11-g39af0d7
On 2011-08-14 00:56, Russ Allbery wrote:
> Niels Thykier <niels@thykier.net> writes:
>
>> Turns out this particular these two were not Perlcritic issues (I
>> applied these fixes from my memory of previous Perlcritic issues).
>
> Oh! Okay. I'm actually surprised on the variable one, as I've seen
> things like that from perlcritic before.
>
It is possible that the warning is there, but was disabled by our
perlcritic settings. Raphael deployed a .perlcriticrc using a whitelist
(and one in t/scripts/critic.t).
>> So the "my vs. our", I seem to have confused "our" with "use vars" in
>> terms of Perlcritic warnings. As for "my vs. our", it is my
>> understanding that "our" makes the variable "public"[0] and it seems to
>> me these variables are not actually intended to be used outside of
>> checks/standards-version.
>
> It does, sort of, in the sense that some other module could access those
> variables from inside the namespace for that check, and my does prevent
> that. So I suppose this is more of a style thing. I always use our for
> global constants or things initialized at startup, combined with the
> all-caps naming convention. I'm not sure if there's an official style, or
> what's the most common.
>
I usually buy the "All caps" as global variables, but I still like to
differ between "exported/public" variables and "internal global" variables.
I personally prefer the checks being completely self-contained and
would like to see shared constants (if any) moved to a separate module
that declares them as exported. Partly because we still have code like:
"""
# If we have an unknown architecture, pretend that all [...]
if ($arch ne 'all' and not exists($ARCH_REGEX{$arch})) {
debug_msg(1, "Unknown architecture: $arch");
$ARCH_REGEX{$arch} = qr/./;
}
""" (checks/binaries)
My second reason is that I like to know that if I want to change (the
format of) a variable, I know I only have to check the current file and
nothing else.
A final note on it, is that we had variables in frontend/lintian
declared as "our" without being all caps... and one of them was actually
used in a check (the --checksums variable).
If we do formalize a code-style, I hope we can agree that this is not
good. :)
> I suppose another alternative would be use constant, at least in some
> cases, although that creates a sub that then doesn't have a type prefix,
> and I think those look kind of strange. But that may just be because I've
> been writing Perl for too long. :)
>
I normally like constants, but some how they feel "wrong" in perl...
like a second-grade citizen or so. To use a constant in a regex you
have to do stuff like "${\CONSTANT}". >.>
>> On the other issue, I thought the "$C, @C"-thing was covered by the
>> "re-use" of same variable in lexical scope, but it turns out it isn't
>> (probably due to the "types" being different).
>
> Ah. Yes. Each type of variable has its own separate namespace.
>
>> I can see the idea behind it now, sort of like a C-string where you
>> can process the string as a whole (strcmp(s, "text")) or individual
>> characters (s[0]). I guess the perl way of doing that is to declare two
>> variables with same name and different type.
>
> Yeah, or at least that's what I've always done in the past.
>
All right, I am ready to buy that approach in moderate doses. Maybe it
is a familiarity thing. Perhaps we can declare both variables at the
same place.
>> In the given case I can see the use of this. That being said, I have
>> my concerns blessing wide-spread use of this. You may remember I got a
>> bit confused with:
>
>> """
>> $checks or ($checks = join(',',keys %check_info));
>> [...]
>> $checks{$c} = 1;
>> """
>> from frontend/lintian in 2.4.3: $checks was a comma-separated list and
>> %checks was a hash where the keys were elements from said list.
>
> Yeah. I'm not sure if this is really confusing, or if it's just a
> familiarity thing. I use the same name (but a different type) for any set
> of variables that all hold exactly the same contents but in a different
> data structure, and use a different variable name if the contents may be
> different.
>
Coming from languages where it is not allowed, it is confusing[0]. :)
But when you have coded in Perl for a while it (also) becomes a
familiarity thing.
It broke one of my "assumptions"; namely a variable is only "declared at
most once in any given scope"[1]. So when I saw "my $checks;" in
frontend/lintian, it never crossed my mind to check for "my %checks;"
which appeared 10-12 lines further down (until Adam pointed it out over
IRC).
In hindsight, I had plenty of pointers to look for a declaration of
%checks (a hashref would have used ->{} rather than {} and strict
forbids/croaks on use of non-hashref as hashref, etc.), which is where
the familiarity part kicks in[2].
For newcomers, I think we can save them a bit of confusion by at least
comment when we do reuse the variable name in these cases.
[0] Even perldata agrees with me: XD
""" This means that $foo and @foo are two different variables. It also
means that $foo[1] is a part of @foo, not a part of $foo. This may seem
a bit weird, but that's okay, because it is weird. """
[1] it may shadow an existing variable like, but there is only one of
them available via the "name" in the scope.
int a;
{
int a; //shadows the original a
}
[2] Of course, back then I was close to declaring myself for stark,
raving mad because the variable acted like a hash and a scalar at the
same time.
>> Can you point me to the section on "my ($v, @v, %v);" in the perl
>> documentation (not really sure what to look/google for here). I would
>> like to get a second look on this. :)
>
> perldata provides some basic explanation of the different data types, the
> namespace separation, and how the data type prefixes work, near the top of
> the documentation page.
>
Thanks, guess I should have read the fine print a bit better. :)
~Niels
Reply to: