Extending a class that uses 'fields'

[Frédéric Perrin]

Hello,

I want to subclass Net::SSH::Expect, by adding a couple of fields to
it. Using "perldoc fields" as a guide, I did the following:

---------------- 8< ----------------

#!/usr/bin/perl

{
package essai;
use base 'Net::SSH::Expect';
use fields qw/ new_field /;
use Data:umper;

sub new {
my $class = shift;
my $self = fields::new($class);
$self->SUPER::new(host => "server.invalid",
password => "password");
print Dumper $self;
$self->run_ssh;
$self->{new_field} = "something";
return $self;
}
}

package main;

my $ssh = essai->new();

---------------- 8< ----------------

But when $self->run_ssh is called, the ->run_ssh method croaks with:

croak(ILLEGAL_STATE . " field 'host' is not set.") unless $host;

We see that the pseudo-hash has all its fields, including new_field,
but there is no data.

Looking at the Net::SSH::Expect->new method, I see that it doesn't
initiate the object in the same way as "perldoc fields" does. I fixed
that by patching the new method in the following way (indentation was
already broken):

# diff -C1 Expect.pm.orig Expect.pm
*** Expect.pm.orig 2010-04-21 09:44:29.000000000 +0200
--- Expect.pm 2010-04-21 09:46:34.000000000 +0200
***************
*** 25,29 ****
sub new {
! my $type = shift;
my %args = @_;
! my Net::SSH::Expect $self = fields::new(ref $type || $type);

--- 25,31 ----
sub new {
! my Net::SSH::Expect $self = shift;
my %args = @_;
! unless (ref $self) {
! $self = fields::new($self);
! }

Is this a genuine bug in Net::SSH::Expect, or am I doing something
wrong?

 

[Frédéric Perrin]

Quoth Frédéric Perrin <[email protected]>:
Use
an inside-out object instead:

use Scalar::Util qw/refaddr/;

my %new_field;

sub new {
my $class = shift;
my $self = $class->SUPER::new(...);
$new_field{refaddr $self} = "something";
}

Be aware that this will fail if your program uses threads. If you can
depend on 5.10, you can use the core Hash::Util::Fieldhash to fix this
(and also avoid the 'refaddr'). You can also use Object::InsideOut,
which works on older perls but is a little heavy for my taste.

Unfortunately, I have to run of RHEL5 with perl 5.8.8. I'm not using
threads, but I don't want to close that door in the future. If I
understand correctly [1], the problem with threads is the refaddr; if
I use an other index that I know is unique between my objects (in my
example, it could be the hostname of the remote server), that will
work, right?

Inside-out objects based on hashes that use the memory address of the
object as the index are not normally thread safe. In a new thread, an
object's memory address will be different and thus all old data
associated with that object will be lost (which also causes a memory
leak).

Otherwise, I'll be using Object::InsideOut, which does pull in a
number of external dependencies.

This is wrong. Net::SSH::Expect->new is a class method, not an object
method, so you should be calling it on the class.

my $self = $class->SUPER::new(...);

I believe (though I haven't tested it) that this will fix your original
problem, as well, since N::S::E->new will now call fields::new with your
classname.

Corrected. However, $self is still empty according to Dumper $self. I
haven't investigated more than that.

Thanks for your time.

 

[Frédéric Perrin]

I want to subclass Net::SSH::Expect, by adding a couple of fields to
it. Using "perldoc fields" as a guide, I did the following:

---------------- 8< ----------------
#!/usr/bin/perl
[...]

Ben Morrow gave you good advice; however, if inside-out objects are
not your thing, Class::Struct makes it easy to create accessor methods
which can be inherited into other classes, e.g.:

IIUC [1], I can't use that to extend Net::SSH::Expect if I don't want
to touch its code. Do I understand correctly ?

 

[PERRIN Frederic]

Corrected. However, $self is still empty according to Dumper $self.
I haven't investigated more than that.

Stupid me. I had simply replaced "$self->SUPER::new" with
"$class->SUPER::new", without saving the results to $self. Of *course*
$self wasn't going to be modified by some class method... But there is
still the possibility you pointed out earlier, with names clashes.

 

[Frédéric Perrin]

If I understand correctly [1], the problem with threads is the
refaddr; if I use an other index that I know is unique between my
objects (in my example, it could be the hostname of the remote
server), that will work, right?

Yes, provided you are *certain* there will never be an overlap.

Yes I am (or rather, the mapping hostname -> my_new_field is well
defined, i.e. a given host will always have the same value for
my_new_field; plus my_new_field is always RO once the object has been
created).

You will also need a DESTROY method to remove dead entries, which I
forgot to mention before. HUF and OIO both handle this for you.

If I lay around an handful of extra copies of my datastructures, that
won't be a big deal, given the nature of the program. It would be more
important to remember to close the SSH sessions when I'm done with
them...

Thank you Ben, and thank you Mumia.