I have a bug, an improvement, and several stylistic
notes.
- First the bug. You are not getting and setting the
element you think you are. Your RE needs to be fixed
to do that.
- The improvement. Your get/set methods are going to
be slow. Usually an AUTOLOAD is best off creating the
method and then calling that so that future invocations
will run faster. This is worth some increase in the
complexity of your method. (Sorry jeffa, they had
not cached their methods properly for that speedup.)
- From here on it, it is all about style. You aren't
using strict. Inside the AUTOLOAD that is often quite
reasonable. But you should at least unimport it there
so that it will work fine when dropped in a module that
does use strict.
- As I have mentioned elsewhere, I am not a fan of making
constructors callable by prototype. I think it is healthy
to distinguish class methods from instance methods. Even
when it wouldn't be overly confusing to allow that, I
don't see a point in allowing it for the simple reason
that the kind of functionality you are likely to want
when constructing a new instance from an old is likely
to be different some day.
- jeffa's point of having an empty DESTROY method is
good.
- I see no reason to create your AUTOLOAD routine by
assigning it to a typeglob. The usual declaration is
enough.
- When you are doing as much as you are (and more when
I get through with it), I see no reason to try to play
golf. Break stuff out a little so people can see what is
going on.
- I don't find myself using get/set methods very much.
If you are putting them in classes mechanically, it is
worth asking why. While I don't entirely agree with
this
article, he does have a lot of good things to say. (I
disagree with his dismissal of the value of
having different views of the same object. I have done
that. I think it is a very valuable thing to do. If
most programmers have not done that, that is only evidence
of the fact that the value of so doing is not better
known. But that is a topic for another day.)
So with that here is a rewritten version:
package PKG;
use strict;
require Carp;
sub new {bless {}, shift;}
sub DESTROY {}
sub AUTOLOAD {
no strict;
if ($AUTOLOAD=~/(\w+)$/) {
my $field = $1;
*{$field} = sub {
my $self = shift;
@_ ? $self->{$field} = shift
: $self->{$field};
};
&$field(@_);
}
else {
Carp::confess("Cannot figure out field name from '$AUTOLOAD'");
}
}
-
Are you posting in the right place? Check out Where do I post X? to know for sure.
-
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big>
<blockquote> <br /> <dd>
<dl> <dt> <em> <font>
<h1> <h2> <h3> <h4>
<h5> <h6> <hr /> <i>
<li> <nbsp> <ol> <p>
<small> <strike> <strong>
<sub> <sup> <table>
<td> <th> <tr> <tt>
<u> <ul>
-
Snippets of code should be wrapped in
<code> tags not
<pre> tags. In fact, <pre>
tags should generally be avoided. If they must
be used, extreme care should be
taken to ensure that their contents do not
have long lines (<70 chars), in order to prevent
horizontal scrolling (and possible janitor
intervention).
-
Want more info? How to link
or How to display code and escape characters
are good places to start.
|