3
\$\begingroup\$

I wrote a wrapper library for a REST API (https://semantics3.com) in Perl using the Moose library. I would like to gather some feedback on it (mainly on the OOP part since I am new to moose), before pushing it out.

GitHub

Base class:

has 'debug'       => (is => 'rw', isa => 'Bool', default => 0);
has 'api_key'     => (is => 'ro', isa => 'Str',    required   => 1);
has 'api_secret'     => (is => 'ro', isa => 'Str',    required   => 1);
has 'api_base'    => (is => 'ro', isa => 'Str',    lazy_build => 1);
has 'oauth_client'          => (is => 'ro', isa => 'Object', lazy_build => 1);    
# Helper methods

method _get {
    my $path = shift;
    my $json_params = shift;
    return $self->_make_request('GET',$path,$json_params);
}

method _make_request {
    my $req_type = shift;
    my $req_path = shift;
    my $req_params_json = shift;

    my $url = $self->api_base . '/' . $req_path;

    #$req->header( Authorization => 
    #    "Basic " . encode_base64($self->api_key . ':'));

    my $resp;
    my $e = eval{
      $resp = $self->oauth_client->request(
        method => $req_type,
        url => $url,
        params => {q => $req_params_json}
      );
    };
    if($@) {
      $self->_req_error_msg("Request could not be made","","");
    }

    if ($resp->code == 200) {
        my $hash = decode_json($resp->content);
        return hash_to_object($hash) if $hash->{object};
        if (my $data = $hash->{data}) {
            return [ map { hash_to_object($_) } @$data ];
        }
        return $hash;
    }
    else {
      $self->_req_error_msg("Request failed",$resp->status_line,$resp->content);
    }

    $e = eval { decode_json($resp->content) };
    if ($@) {
      $self->_req_error_msg("Could not decode HTTP response",$resp->status_line,$resp->content);
    };

    #warn "$e\n" if $self->debug;
    die "Error occured\n";
}

method _req_error_msg {
  my $msg =shift;
  my $status_line = shift;
  my $content = shift;

  print STDERR "Message: $msg\n";
  print STDERR "Status Line: $status_line\n";
  print STDERR "Content: $content\n";
}

sub hash_to_object {
    my $hash = shift;
    my $class = 'Net::Semantics3::' . ucfirst($hash->{object});
    return $class->new($hash);
}

method _build_api_base { 'https://api.semantics3.com/v1' }

method _build_oauth_client {
    my $ua = LWP::UserAgent->new;
    $ua->agent("Semantics3 Perl Lib/$VERSION");
    my $oauth_client = OAuth::Lite::Consumer->new(
      ua => $ua,
      consumer_key => $self->api_key,
      consumer_secret => $self->api_secret,
    );
    return $oauth_client;
}

Products.pm

extends 'Net::Semantics3';
use constant MAX_LIMIT => 10;

has 'products_query' => (isa => 'HashRef', is => 'rw', default => sub { my %hash; return \%hash; } );
has 'categories_query' => (isa => 'HashRef', is => 'rw', default => sub { my %hash; return \%hash; } );
has 'query_result' => (isa => 'HashRef', is => 'rw', default => sub { my %hash; return \%hash; } );
has 'sem3_endpoint' => (isa => 'Str', is => 'ro', writer => 'private_set_endpoint', default => "products");

Products: {

  #stupid hack to enforce method overloading
    method field {
        my $field_name = shift || die "A Field Name is required";
        my $field_value1 = shift;
        my $field_value2 = shift;
        if(!defined($field_value1) && !defined($field_value2)) {
          die "A Field value is required";
        }
        if(defined($field_value2)) {
          if(!defined($self->products_query->{$field_name})) {
            $self->products_query->{$field_name} = {};
          }
          $self->products_query->{$field_name}->{$field_value1} = $field_value2;
        } else {
          $self->products_query->{$field_name} = $field_value1;
        }
    }

    method get_products {
      $self->_run_query("products",$self->products_query);
      return $self->query_result();
    }
}
\$\endgroup\$
2
\$\begingroup\$

In Net::Semantics3 _make_request I wasn't too sure about the error handling. It looks like _req_error_msg doesn't die if any part of the request process fails. Also 200 may not be the only valid HTTP status code, it's safer to use the is_success method

if ($resp->is_success) { #handle response...

You could also clean up the eval calls and use Try::Tiny instead, which I think looks a bit neater.

My Moose is a bit rusty, but I believe you can use class names in type definitions (feel free to correct if this isn't the case)

has 'oauth_client' => (is => 'ro', isa => 'OAuth::Lite::Consumer', lazy_build => 1);  

As for the general structure OO structure I would probably have preferred separate classes for representing queries, results and the Net::Semantics3 user agent. Almost analogous to HTTP::Request, LWP::UserAgent and HTTP::Response. If I was using your code I'd like to be able to do something like this:

my $product_query = Net::Semantics3::ProductsQuery->new({brand => "Toshiba television"});
my $net_semantics3_ua = Net::Semantics->new();
my $prod_resultset = $net_semantics3_ua->execute_query($product_query);
foreach my $prod($prod_resultset->all){
    say $prod->field_name;
}

There's nothing really wrong with the approach you've taken, but if I read the example code in your test.pl script it doesn't feel like the interface is very perlish. I think you could make it a bit more intuitive for programmers using your API

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.