Re: Very very small style question
by merlyn (Sage) on Dec 18, 2000 at 01:08 UTC
|
Or as another alternative:
sub method {
my $self = shift;
my $i = shift;
my $old = $self->{B}[$i];
$self->{B}[$i] = shift if @_;
$old;
}
I tend to hate magic numbers. What if you decide later that there should
be another parameter between $self and $i?
-- Randal L. Schwartz, Perl hacker | [reply] [d/l] |
Re: Very very small style question
by maverick (Curate) on Dec 18, 2000 at 02:47 UTC
|
Another approch along the lines of Randal's comment is the
way I tend to pass params, using a hash ref.
Consider this scenario. Say we have some parent object that
is going to pass of some work to some number of child objects.
How many child objects? Don't know; more could be added later
depending on the task. Say some method in most of the children
require $foo, $bar, and $baz. You could write them like:
sub my_sub {
my $self = shift;
my $foo = shift;
my $bar = shift;
}
Say one of these child objects doesn't require $foo for it's,
task. You could special case the call to my_sub in that child,
but that breaks the idea of the parent being generic and the
children only being specialized. So you just live with shifting
off the useless $foo in that child...which isn't so bad.
But, say later down the line, we want to write this new child
that also needs $bork to be passed into that method. So, depending
on how much stuff you have to pass around, this could quickly become
a mess.
Thus I use a hashref to bypass a lot of this.
sub my_sub {
my $self = shift;
my $params = shift;
my $foo = $params->{'foo'};
my $bar = $params->{'bar'};
}
# and in another child we could have
sub my_sub {
my $self = shift;
my $params = shift;
my $foo = $params->{'foo'};
my $bork = $params->{'bork'};
}
# and the parent calls them like
$child->my_sub({'foo' => $foo,
'bar' => $bar,
'baz' => $baz,
'bork' => $bork});
So, there's no remembering the order of the params, no
shifting variables you don't need, and passing a new one
won't break existing children, since they only pull out the ones
they need, and never see the rest. The only thing you have
to remember is which key has the value you need.
/\/\averick | [reply] [d/l] [select] |
Re: Very very small style question
by chromatic (Archbishop) on Dec 18, 2000 at 08:46 UTC
|
I'd actually prefer separate get/set methods, in a case like this. It's probably just me, but I'd like to be able to pass a list of parameter names to CGI::param() and get back their values.
If I were to do this, I'd probably do just a bit more to make it obvious:
sub method {
my ($self, $i) = @_;
if (@_ > 2) {
$self->{B}[$i] = $_[2];
}
return $self->{B}[$i];
}
| [reply] [d/l] |
Re: Very very small style question
by mdillon (Priest) on Dec 18, 2000 at 01:00 UTC
|
i actually prefer the following:
sub method {
my ($self, $i, $new) = @_;
my $old = $self->{B}[$i];
$self->{B}[$i] = $new unless @_ < 3;
return $old;
}
| [reply] [d/l] |
Re: Very very small style question
by Maclir (Curate) on Dec 18, 2000 at 02:02 UTC
|
I think there are two parts to this, the first is the use of modifiers - basically, what it preferable; an "if COND" or and "unless NOT COND". The unless has an implicit not - since you want to perform the statement if the third paramater was provided.
My personal view is the "if @ >=3" if preferable to the "unless" option. We are wanting to do something if there are still some paramaters to process - so write it that way. Whether the ">2" or ">=3" is better, well, that to me is personal choice. However, the ">2" option is the minimalist solution - we don't care how many paramaters there are, as long as there is at least one more.
The second issue is the one Merlyn raised - what is the most desirable to make processing decisions on subroutine paramaters? Merlyn's way - doing a shift on @_ and processing each paramater in turn is probably the safest way. Each paramater has its processing "self containted" - that is, you are not relying on its position in the @_ array, or the total number of arguments, or other potential time bombs.
| [reply] |
Re: Very very small style question
by Fastolfe (Vicar) on Dec 18, 2000 at 05:00 UTC
|
Disregarding for the moment the alternatives of doing what you're describing, when dealing with numerical constants in my code, I tend to use the number that makes the most sense in the situation, and adapt my tests around that. If I'm checking for 3 arguments, I want to try and use '3' as my constant (thus requiring me to use >= instead of >), not 2. For readability reasons, the use of a 2 seems less obvious to someone coming to the code later. | [reply] [d/l] [select] |
Re: Very very small style question
by blogan (Monk) on Dec 18, 2000 at 05:37 UTC
|
I prefer either one of these two:
sub method {
my ($self, $i, $new) = @_;
my $old = $self->{B}[$i];
#make sure we have correct number of parameters
$self->{B}[$i] = $new if @_ > 2;
return $old;
}
or
sub method {
my ($self, $i, $new) = @_;
my $old = $self->{B}[$i];
#make sure we have correct number of parameters
$self->{B}[$i] = $new if @_ >= 3;
return $old;
}
Couldn't you do an (if defined $new) also? | [reply] [d/l] [select] |
|
The one reason for checking the size of @_, rather than whether $_[2] is defined, is that you may actually want to allow someone to call the method with undef as an argument.
| [reply] |
|
| [reply] |
(tye)Re: Very very small style question
by tye (Sage) on Dec 18, 2000 at 21:39 UTC
|
Well, I'd probably help my user by catching some coding
mistakes rather than letting them cause strange behavior
that they'd have to spend much more time debugging:
sub method {
my $self= shift(@_);
croak 'Usage: $oldVal= $obj->method( $idx [, $newVal ] )',$/
unless 1 <= @_ && @_ <= 2;
my $i= shift(@_);
my $old= $self->{B}[$i];
$self->{B}[$i]= shift(@_) if @_;
return $old;
}
But I don't like the magic numbers there so I might avoid
them at the cost of some duplication but with even better
error messages:
sub method {
my $self= shift(@_);
croak 'Missing $idx; usage: $oldVal= $obj->method( $idx [, $newVal
+ ] )',$/
unless @_;
my $i= shift(@_);
my $old= $self->{B}[$i];
$self->{B}[$i]= shift(@_) if @_;
croak 'Too many args; usage: $oldVal= $obj->method( $idx [, $newVa
+l ] )',$/
if @_;
return $old;
}
Then you can remove some duplication with:
{
my $usage;
BEGIN { $usage= '$oldVal= $obj->method( $idx [, $newVal ] )' }
sub method {
my $self= shift(@_);
croak "Missing \$idx; usage: $usage\n" unless @_;
my $i= shift(@_);
my $old= $self->{B}[$i];
$self->{B}[$i]= shift(@_) if @_;
croak "Too many args; usage: $usage\n" if @_;
return $old;
}
}
-
tye
(but my friends call me "Tye") | [reply] [d/l] [select] |
|
Why, in your last suggestion, do you use a BEGIN block? Are there any circumstances where $usage would not be set if you hadn't placed the assignment to it in a BEGIN block?
| [reply] |
|
I always assume that subroutines will be put into a scope
where they will be compiled and never run [ that is,
the subroutine will probably be called, but the
lines around the subroutine will never be run ].
If you don't use the BEGIN, then you have a race condition
because the subroutine exists and can be called as soon as
it is compiled but its "static variable" (will be declared
but) won't be initialized until that line gets run,
which could happen much later or not at all.
I first ran into this problem with some border cases of
using modules. The one that I remember was having two
modules that depend on each other. Perl actually manages
to get this mostly right (doing better than I would have
thought was even possible). But the effect is that if A.pm
uses B.pm which uses A.pm, then either A::import() or
B::import() will be called before the code for that module
gets run. I recall finding another case that was less easy
to justify ignoring, but I don't remember the specifics
right now.
Another case is self-inflicted. I hate having to read
"between" a huge list of subroutine declarations looking
for the run-time statements so I force my top-level run-time
code into sub main and enforce this discipline
by ending my global declarations with:
exit( main( @ARGV ) );
(see (tye)Re: Stupid question for other self-inflicted discipline).
So I guess that boils down to "No, I don't have any glaring,
screaming, obvious cases that make not doing this a really,
really bad idea." (: It is a personal coding habit that
has saved me time more than once. It avoids a race
condition, which is usually a good thing.
-
tye
(but my friends call me "Tye") | [reply] [d/l] [select] |
Re: Very very small style question
by jynx (Priest) on Dec 19, 2000 at 01:37 UTC
|
And on an even smaller sidenote:
i usually prefer having a blank line between the subroutine
body and the return call so that:
a) know exactly where it's returning, easy for maintenance
b) know exactly what it's returning, easy for debugging
These are just my preferences...
jynx | [reply] |
Re: Very very small style question
by gryphon (Abbot) on Dec 19, 2000 at 02:11 UTC
|
I heard from a wise old Perl guru once that using >= or <= rather than > or < is less efficient. I have no idea whether this is true or not, but I have no reason to doubt it. The wise guru's point was that >=/<= was in fact two operations or took longer to process than >/<.
Anyone know if this is true? I'd be very interested to find out...
-Gryphon.
| [reply] |