Dermot wrote:
>
> Phew.
>
> I thought I was loosing my marbles there. I was staring at the screen
> for ages trying to find a difference. To be fair, I think the
> semi-colon were my balls-up :-/
>
> Yes Rob I am trying to do some OO. It might make sense for what I'm
> after (upload different type of data to a remote server). I am making
> my way but I think I'll be using the list a bit over the next few
> days.
>
> I think I have harnessed some auto vivification as per Shawn email.
>
> I'll show my hand now in case anyone has comments on how to improve my
> efforts.
>
> In particular I wouldn't mind finding a way to have the 'type'
> subroutine/method be able to return (get) $self's type (EG: typeOne,
> typeTwo...etc) or set $self's type. Or should I separate the getting
> and setting out?
>
> TiA,
> Dp.
>
>
> my %_HASH = (
> typeOne => {
> root => '/path/to/typeOne',
> },
> typeTwo => {
> root => '/path/to/typeTwo',
> }
> );
>
> sub new {
> my $class = shift;
> my $self = [];
> $self->[0] = {};
> bless($self, $class);
> return $self;
> }
>
> sub type {
> my $self = shift;
> if (@_) {
> # Setting the type
> if (exists $_HASH{$_[0]}) {
> $self->[0]->{type} = $_HASH{$_[0]};
> $self->[0]->{logfile} =
> $_HASH{$_[0]}->{root}.'/'.$_[0].'.log'; # Logfile
> }
> else {
> croak "The type arg \"$_[0]\" is not in the known
> types: ", join ' ', keys %_HASH,"\n";
> }
> }
> else {
> # Getting the type
> $self->[0]->{type};
> }
> }
OK great.
First of all the semicolons weren't your problem - they stop the code compiling
altogether so you would have no symptoms to report. You must have posted a wrong
version like Yitzle.
Now I could try and guess your intentions and rewrite your code, but I would
guess wrongly and you wouldn't get the experience of working on it yourself. So
here are some observations.
- The usual reason for using a variable name with a leading underscore is to
denote that it's a private variable, but lexical variables aren't visible
outside the package anyway so you may as well drop the underscore and make it
more readable. You should also call it something better than HASH because the %
says that, so how about %TYPES?
- It's very unusual to bless an array reference for use as an object. I think
you should use a hash reference, and if you have array-like core data just make
one of the hash values an anonymous array.
- In a similar vein to the last point, at present you have $self->[0] all over
the place. Is there ever going to be a $self->[1]? It doesn't look like it from
the way you've written your methods, and you seem to have a superfluous array
layer in the data that just causes noise in the code.
- Remember that any pair of }{ or ][ or a mixture of the two doesn't need an
intervening ->
- To answer your question, yes it's a good idea to write accessor methods like
type so that they will both read and write a value. But I would guess that you
shouldn't be able to modify an object's type once it has been created?
If you were to drop the spare array indirection as I suggested and rename your
hash to %TYPES you could write
sub type {
my $self = shift;
if (@_) {
my $newtype = shift;
$self->{type} = $newtype;
$self->{logfile} = "$TYPES{$newtype}{root}/$newtype.log";
}
return $self->{type};
}
which changes the type if a new one is supplied and then returns the current
value.
See how you get on with that.
HTH,
Rob
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
