4
\$\begingroup\$

I wrote a wrapper library for a REST API 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.

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();
 }
}
asked Oct 3, 2012 at 7:44
\$\endgroup\$

1 Answer 1

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

answered Oct 3, 2012 at 20:34
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.