Why isn't shift the same as $_[0]?

[marcin-usenet]

See the following test program. I expected that to print "666" four
times. However it does it three times, and then says Use of
uninitialized value in print at ./test line 56.

Can anyone explain why? Noone felt like explaining in #perl on
freenode. :-(

regards

Marcin

#!/usr/bin/perl -w
use strict;
package Base;
sub new
{
my $that = shift;
my $class = ref $that || $that;
my $self = {};
bless $self, $class;
return $self;
}

use vars '$AUTOLOAD';
sub AUTOLOAD
{
my $self = shift;
my $name = $AUTOLOAD;
$name =~ s,.*:,,;
# scalar mutator
return $self->{$name} = shift if @_;
# accessor
return $self->{$name};
}

package Hash;
use base 'Base';

package Container;
use base 'Base';
sub setting_with_shift
{
my $self = shift;
wantarray ? map { $self->settings->{$_} } @_ :
$self->settings->{shift};
}
sub setting_without_shift
{
my $self = shift;
wantarray ? map { $self->settings->{$_} } @_ :
$self->settings->{$_[0]};
}


package main;

my $h = Hash->new;
$h->foo(666);

my $c = Container->new;
$c->settings($h);

print $c->setting_without_shift('foo');
print "\n";
print $c->setting_with_shift('foo');
print "\n";
print scalar $c->setting_without_shift('foo');
print "\n";
print scalar $c->setting_with_shift('foo');
print "\n";

 

[Fabian Pilkowski]

See the following test program. I expected that to print "666" four
times. However it does it three times, and then says Use of
uninitialized value in print at ./test line 56.

Without reading your code, I'll answer your question ;-)

Can anyone explain why? Noone felt like explaining in #perl on
freenode. :-(

shift() without params removes the first value from @_ (resp @ARGV) and
returns it. Hence, the array @_ is shortened. When using $_[0] directly
you get the first element of @_ without removing it from @_. The array
@_ stays unchanged.

Please, read `perldoc -f shift` to get the details.

regards,
fabian

 

[porridge]

Without reading my code, you missed the point. I know that shift
modifies the array. But this is irrelevant in this case, since the
array is abandoned immediately afterwards anyway.

Marcin

 

[A. Sinan Unur]

(e-mail address removed) wrote in @g43g2000cwa.googlegroups.com:

See the following test program. I expected that to print "666" four
times. However it does it three times, and then says Use of
uninitialized value in print at ./test line 56. ....

#!/usr/bin/perl -w

use warnings;

is better because it allows you to turn selected warnings on/off in a
lexical scope. See

perldoc perllexwarn

use strict;
package Base;

Not a good name.

See

perldoc Base

sub new
{
my $that = shift;
my $class = ref $that || $that;

I know this technique is shown in various places, but I consider it bad
style. What is the point of calling 'new' on an existing object?

my $self = {};
bless $self, $class;
return $self;
}

use vars '$AUTOLOAD';
sub AUTOLOAD
{
my $self = shift;
my $name = $AUTOLOAD;
$name =~ s,.*:,,;

There is no point in not using s/.*:// unless you are purposefully
trying to trip up your reader.

$name = substr($name, rindex($name, ':') + 1);

is better in IMHO.

sub setting_with_shift
{
my $self = shift;
wantarray ? map { $self->settings->{$_} } @_ :
$self->settings->{shift};

Ahem, you are using shift as a hash key, not calling shift on @_:

$self->settings->{ shift() };

The rest snipped.

Here is a modified script that does not give the warning.

#! /usr/bin/perl

use strict;
use warnings;

package My::Base;

sub new { bless { }, $_[0] }

use vars '$AUTOLOAD';
sub AUTOLOAD {
my $self = shift;
my $name = substr($AUTOLOAD, 1 + rindex($AUTOLOAD, ':'));
return $self->{$name} = shift if @_;
return $self->{$name};
}

package My::Hash;
use base 'My::Base';

package My::Container;
use base 'My::Base';

sub setting_with_shift {
my $self = shift;
wantarray
? map { $self->settings->{$_} } @_
: $self->settings->{ shift() };
}

sub setting_without_shift {
my $self = shift;
wantarray
? map { $self->settings->{$_} } @_
: $self->settings->{$_[0]};
}

package main;

my $h = My::Hash->new;
$h->foo(666);

my $c = My::Container->new;
$c->settings($h);

print $c->setting_without_shift('foo')."\n";
print $c->setting_with_shift('foo')."\n";
print $c->setting_without_shift('foo')."\n";
print $c->setting_with_shift('foo')."\n";

__END__

Sinan

 

[A. Sinan Unur]

Without reading my code,

Who did not read the code?

you missed the point.

Who missed the point?

Please provide some context in your posts. I just posted a response to
your question, I hope this is not in response to that, but I do not see
any other posts in this thread yet.

Please read the posting guidelines to learn how to adopt an effective
follow-up style.

Please read the discussions in this newsgroup regarding posting from
Google.

Sinan

 

[Anno Siegel]

Without reading my code, you missed the point. I know that shift
modifies the array. But this is irrelevant in this case, since the
array is abandoned immediately afterwards anyway.

Then, please, do a better preparatory job to isolate the problem.

You are using an AUTOLOAD-infested hierarchy of classes, some 60 lines of
that, to show the behavior. I believe the problem could be demonstrated
in a simpler setup. Is AUTOLOAD essential? Is inheritance? Reduce
the example until the effect goes away, then add back on what's needed
to show it again. That way you arrive at a minimal example. Often
the process clears up the problem by itself.

Yes, that's work. If you don't do it, we will have to. That lowers
your chances for a useful reply.

Anno

 

[Anno Siegel]

(e-mail address removed) wrote in @g43g2000cwa.googlegroups.com:

[snip good job of finding the problem, except:

I know this technique is shown in various places, but I consider it bad
style. What is the point of calling 'new' on an existing object?

I concur. There is a place for methods that are both class and object
methods (most notably those that ignore their first argument entirely),
but routinely making ->new object-callable just because we can is bad
style. The role of $obj is entirely clear in

ref( $obj)->new();

but in

$obj->new();

we can only guess.

Anno

 

[Anno Siegel]

(e-mail address removed) wrote in @g43g2000cwa.googlegroups.com:

[snip most of good job of finding the problem]

I know this technique is shown in various places, but I consider it bad
style. What is the point of calling 'new' on an existing object?

I concur. There is a place for methods that are both class- and object
methods (most notably those that ignore their first argument entirely),
but routinely making ->new object-callable just because we can is bad
style. The role of $obj is entirely clear in

ref( $obj)->new();

but in

$obj->new();

we can only guess.

Anno

 

[porridge]

Then, please, do a better preparatory job to isolate the problem.

Sorry. This was already a result of trimming an over 1000-line set of
modules, so I was under an impression that this was enough

Marcin

 

[porridge]

(e-mail address removed) wrote in @g43g2000cwa.googlegroups.com:


use warnings;

Nice. Somehow I missed this pragma. Maybe it was introduced after I
started learning perl (that was some 6 years ago..).

Not a good name.

See

perldoc Base

porridge@melina11:~$ perldoc Base
No documentation found for "Base".

I know this technique is shown in various places, but I consider it bad
style. What is the point of calling 'new' on an existing object?

To get another object of the same class? That's handy sometimes.

There is no point in not using s/.*:// unless you are purposefully
trying to trip up your reader.

$name = substr($name, rindex($name, ':') + 1);

is better in IMHO.

I guess that's a matter of taste. The regex way seems much clearer and
terse to me. And that way I don't need to remember about that "+1".
Also, it's taken straight from man perltoot.

Ahem, you are using shift as a hash key, not calling shift on @_:

Ah!

regards,

Marcin

 

[Sherm Pendley]

A. Sinan Unur wrote:

porridge@melina11:~$ perldoc Base
No documentation found for "Base".

It's case-sensitive - 'perldoc base'.

If you name your package "Base", it will cause problems on
case-insensitive filesystems such as NTFS (Windows) or HFS+ (Mac OS X).

To get another object of the same class? That's handy sometimes.

It's ambiguous. Will calling new() as an object method create a copy of
the existing object, or create a "blank" instance?

Using two distinct methods, a class method new() and an object method
copy(), leads to clearer code that's easier to understand.

sherm--

 

[A. Sinan Unur]

porridge@melina11:~$ perldoc Base
No documentation found for "Base".

Fine. But, all sorts of funny things can happen when using a
case-insensitive operating system.

To get another object of the same class? That's handy sometimes.

A fresh object, or one that is a clone of the other? Is it a deep copy
or shallow copy? Etc etc. It makes it easier for readers of the code to
comprehend what's happening when 'new' means 'new', and there is no
scope for confusion. When I see:

my $tmpl = HTML::Template->new;

I know the type of $tmpl at the type of invocation, as well as what I am
getting in return.

When I see:

my $tmpl = $other->new;

I have no idea what $other->new is giving me unless I go into the
documentation.

This point still holds.

I guess that's a matter of taste. The regex way seems much clearer and
terse to me.

Your regex is anything but clear. First off, why not use s///? The
difference between a , and a . is not that easy to discern.

Second, I do not intend to push my substr preference unduly here, but
why invoke the whole regex machinery on every call to AUTOLOAD, which,
in your case, will be many. Of course, for a conclusive argument, one
would need to benchmark it

And that way I don't need to remember about that "+1".
Fine.

Also, it's taken straight from man perltoot.
So?


Ah!

You could have, of course, figured this out by yourself, if you had
tried to come up with a minimal example exhibiting the problem. And, you
have no thanks to give to boot.

Hmmm.

Sinan

 

[phaylon]

When I see:

my $tmpl = $other->new;

I have no idea what $other->new is giving me unless I go into the
documentation.

Hm, to say about this one: I find it relatively clear that a new method
returns a new object, not a clone etc, a new one. The "context" says if we
want a new one by object, or by package. Very handy.

Besides that, you should consult the docs anyway, who knows what
parameters new is taking? What else is it doing?

Third, it is listed in perldoc perltoot on "Planning for the Future:
Better Constructors", so it should be some kind of known technique.

just my 2c,
p

 

[Anno Siegel]

Hm, to say about this one: I find it relatively clear that a new method
returns a new object, not a clone etc, a new one. The "context" says if we
want a new one by object, or by package. Very handy.

I have seen at least three behaviors of $obj->new( ...):

- It ignores $obj altogether, only the parameters count
- It ignores the parameters altogether and clones $obj
- It does a combination of both, taking the values for missing
parameters from the object.

I bet other behaviors have been implemented too.

Besides that, you should consult the docs anyway, who knows what
parameters new is taking? What else is it doing?

If you know what Class->new( ...) does you also know what
ref( $obj)->new( ...) does. If ref() is hidden inside ->new, you
must look again. That's why I prefer the explicit style.

Third, it is listed in perldoc perltoot on "Planning for the Future:
Better Constructors", so it should be some kind of known technique.

Oh, quite a few heavy-weight Perl writers have endorsed the idea
of making ->new an object method. They are wrong.

Anno

 

[Anno Siegel]

Hm, to say about this one: I find it relatively clear that a new method
returns a new object, not a clone etc, a new one. The "context" says if we
want a new one by object, or by package. Very handy.

I have seen at least three behaviors of $obj->new( ...):

- It ignores $obj altogether, only the parameters count
- It ignores the parameters altogether and clones $obj
- It does a combination of both, taking the values for missing
parameters from the object.

I bet other behaviors have been implemented too.

Besides that, you should consult the docs anyway, who knows what
parameters new is taking? What else is it doing?

If you know what Class->new( ...) means you also know what
ref( $obj)->new( ...) means. If ref() is hidden inside ->new, you
don't. That's why I prefer the explicit style.

Third, it is listed in perldoc perltoot on "Planning for the Future:
Better Constructors", so it should be some kind of known technique.

Oh, quite a few heavy-weight Perl writers have endorsed the idea
of making ->new an object method. They are wrong.

That is not to say that ->new should never be object-callable. It may
make sense in some contexts. But doing it routinely is right there
with never using "keys %h" other than "sort keys $h". It's just a
bad habit.

Anno

 

[phaylon]

I have seen at least three behaviors of $obj->new( ...):

Well, I use only one, find that clear, document it in my code.

I bet other behaviors have been implemented too.

Sure

If you know what Class->new( ...) means you also know what ref(
$obj)->new( ...) means. If ref() is hidden inside ->new, you don't.
That's why I prefer the explicit style.

Yep, but do you just make an cpan-install and go for

use NewPackage;
my $np = new NewPackage;

without reading the docs? If it's there, it should be documented. I would
rather blame the documentation if not, not the code itself.

That is not to say that ->new should never be object-callable. It may
make sense in some contexts.

Yep. And we never know what this contexts may be like. So why not
implement it, document it, and let the ones that want to use it, use it?

But doing it routinely is right there with
never using "keys %h" other than "sort keys $h". It's just a bad habit.

Well, I'm with you here, but have to say again, that I don't see any bad
style here, if it's well done.

 

[A. Sinan Unur]

....

Besides that, you should consult the docs anyway, who knows what
parameters new is taking? What else is it doing?

I meant "further the source" rather than "documentation".

Suppose you are reading somescript, and around line umpteen, you run
into a line that says:

my $other_tmpl = $tmpl->new;

There is no way for me to know, just by looking at this line, what type
of object $tmpl. I need to go find where it was initialized. Then, I
need to figure out in what way, if any, the instance constructor for
that class differs from the class constructor.

I have a simple mind.

Third, it is listed in perldoc perltoot on "Planning for the Future:
Better Constructors", so it should be some kind of known technique.

And the OP had blindly copied his code from somewhere. It is better not
to add features "just in case". If, one day, it turns out that having an
instance new, and not a clone, copy, clean_copy, or some such method,
the change is trivial to implement in a way that does not break existing
code that depends on class new.

Anyway, I do not wish to act like a troll here, and go on and on about
style issues. I am not the originator of any of the ideas above. It is
just my judgement, having seen various opinions, that having an instance
new needs to be justified on a case-by-case basis rather than being
blindly copied from perltoot.

In short I like:

Just because .bless allows an object to be used for a class
doesn't mean your new constructor has to do the same. Some
folks have philosophical issues with mixing up classes and
objects, and it's fine to disallow that on the constructor
level. In fact, you'll note that the default .new above
requires a Class as its invocant. Unless you override it,
it doesn't allow an object for the constructor invocant.
Go thou and don't likewise.

<URL: http://www.perl.com/pub/a/2004/04/16/a12.html?page=7
#object_construction>

Sinan

 

[phaylon]

Suppose you are reading somescript, and around line umpteen, you run
into a line that says:

my $other_tmpl = $tmpl->new;

There is no way for me to know, just by looking at this line, what type
of object $tmpl. I need to go find where it was initialized.

Ah, now I got it. Well, sure you're right. But I'd then (if we're in
unknown code, not in ours) blame the coder of this snippet. There should
be a comment right above saying why this is done this way.

You may see, I have absolutely no problems with coding styles that are
different from mine, if the coder knew what he did. But I'm sure we agree
on this, but maybe not on "knew what he did".

I have a simple mind.

Who hasn't?

And the OP had blindly copied his code from somewhere. It is better not
to add features "just in case".

I'm with you here, but isn't that more a matter of "keep it straight and
simple"? Hm. Are you arguing against the bless {}, ref($x) || $x or
against the fact, that it's there? If the second, I think I just
misunderstood you. Sorry if so.

Anyway, I do not wish to act like a troll here, and go on and on about
style issues.

To be honest, this is why I like this community.

Well, I think we made ourselves clear.

 

[A. Sinan Unur]

A. Sinan Unur wrote:
....

I'm with you here, but isn't that more a matter of "keep it straight
and simple"? Hm. Are you arguing against the bless {}, ref($x) || $x
or against the fact, that it's there? If the second, I think I just
misunderstood you. Sorry if so.

I am arguing more against the latter than the former. But, I do not
think the former should be part of new unless there is a
good/justifiable reason for it. I am not categorically opposed to it,
but I am not likely to include an instance new in code I write.

Well, I think we made ourselves clear.

We did. Thanks for the insight.

Sinan