Need a second set of eyes: Very odd failure

[blue_cowdawg]

I must be missing something here so I'm asking for more eyes on something.

I built a fairly simple straightforward module here and yet it is failing to work. This module does curls to an API (The Foreman) to extract some data from it. Nothing fancy here.... Here are the most important bits:

package NCS::PuppetDB::Data;

use 5.010000;
use strict;
use warnings;
use JSON;

our $curl="/usr/bin/curl -G 'http://localhost:8080/v3";
our $VERSION = '0.5.2';

sub new {
    my $self={};
    bless $self,"NCS::PupptDB::Data";
    return $self;
}

sub getHosts {
    my $self = shift;
    my $cmd=sprintf("%s/nodes 2>/dev/null",$curl);
    my $json = JSON->new->allow_nonref;
    my $jsonData=`$cmd`;
    my $decoded = $json->decode($jsonData);
    my $retval=[];
    for my $entry(@$decoded){
    push @$retval,$entry->{name};
    }

    return sort $retval;
}

sub getHostFacts {
    my $self = shift;
    my $host = shift;

    my $cmd=sprintf("%s/nodes/%s/facts' 2>/dev/null",$curl,$host);
    my $json = JSON->new->allow_nonref;
    my $jsonData=`$cmd`;
    my $decoded = $json->decode($jsonData);
    my $retval = {};

    for my $fact(@$decoded){
    $retval->{$fact->{name}} = $fact->{value};
    }

    return $retval;
}

sub getFact {
    my ($self,$host,$factname)=@_;
    my $facts=getHostFacts($host);
    return $facts->{$factname};
}

1;

The one and only script using it (so far) is complaining:

Can't locate object method "getHosts" via package "NCS::PupptDB::Data"

What am I missing?

[haukex]

bless $self,"NCS::PupptDB::Data";
                     ^^ missing "e"

Update: Wow, stevieb and I posted within 2 seconds of another :-)

The usual pattern would be:

sub new {
    my $class = shift;
    my $self = {};
    bless $self, $class;
    return $self;
}

Makes subclassing easier (and avoids this typo ;-) )

[blue_cowdawg]

ack! My eyes must be going on me... THANKS!!!

[haukex]

No problem :-) The only reason I found it so quickly was because for testing I didn't feel like creating the subdirectories and was going to rename the package from NCS::PuppetDB::Data to NCS_PuppetDB_Data, and I was surprised when my "find" didn't match that line... so it was just laziness ;-)

[stevieb]

It happens to everyone :)

I did not have time at that moment to write out a full blown answer, but haukex's feedback is definitely the way to go when doing native OO in Perl for certain.

[karlgoethebier]

"...My eyes..."

To be honest: i didn't see it too. I guess i need new glasses ;-)

[stevieb]

NCS::PupptDB::Data
---------e--------

[karlgoethebier]

BTW, why don't you use a plain old module? I don't see any reason to OO here.

package NCS::PuppetDB::Data {

    use strict;
    use warnings;
    use JSON::Tiny qw(decode_json encode_json);
    use Net::Curl::Easy qw(:constants);
    use Exporter qw( import );

    our @EXPORT_OK = qw( getHosts getHostFacts getFact);
    our $VERSION = '0.5.3';

    sub getHosts {
        ...
    }

    sub getHostFacts {
        ...
    }

    sub getFact {
        ...
    }

    1;
}

Please correct me if i miss something. I added some of my favorite modules. No need to shell out for curling etc.

Update: But perhaps this layout is interesting for you:

Read more... (2 kB)

Best regards, Karl

[AnomalousMonk]

Ditto regarding karlgoethebier's query on the propriety of OO here. And if OO is needed, didn't you get into trouble in the first place by violating the DRY principle? bless blesses into the current package by default anyway, so one might just have written
    bless $self;
in the  new() constructor definition, or maybe
    bless $self, __PACKAGE__;
if one wanted to be very explicit.

But an important aspect of OO is class inheritance, and to support future inheritance capability, shouldn't one write the constructor, assuming this is a base class, as something like:

sub new {
  my $class = shift;
  ;;
  my $object = $class->initialized_object(@_);
  return bless $object => $class;
  }

(Of course, now you need to define an  initialized_object() method, but that's life in OO-land.)

[RMGir]

Aside from the bless issue everyone else spotted (but I would have TOTALLY missed), isn't there something strange going on here?

our $curl="/usr/bin/curl -G 'http://localhost:8080/v3";

# ...

sub getHosts {
    my $self = shift;
    my $cmd=sprintf("%s/nodes 2>/dev/null",$curl);

Aren't you going to wind up with a mismatched single quote in $cmd if 'getHosts' gets called? 'getHostsFacts' does it right, it looks like...