After a very quick glance... the use_ok tests should only be specified in a single file (00-load.t). Having them in each test file is redundant, and if/when you get up to 30-50 test files (often by copy/pasting, then editing), that's a lot of test calls that are unnecessary.

Also, in my tests, I prefer to not use the plan tests => N, and instead, forget that and just put done_testing(); at the end of my test. This allows me to not worry about keeping track of my test count (this is just my personal preference though).

The TEST_CONFIG.tpl file should reside in the t/ directory (perhaps t/data, and be automatically loaded into the env var when needed. This keeps the directory structure clean, and alleviates a user from having to do manual work when running/adding tests.

Things like this:

foreach my $key (@PARAMS) { $self->{$key} = $params{$key} || '' }

...may be better written like:

$self->{$key} = defined $params{$key} ? $params{$key} : '';

The former way will assign an empty string to the value of $self->{key} if $params{$key} has a false value, which may not be what you want (or, you may want to change in the future). The latter way will only assign an empty string if the value is undefined. I could write:

$self->{$key} = $params{$key} // '';

...but you're using minimum perl 5.6, and the defined-or (//) operator wasn't released until (iirc) 5.10

Here's another personal preference. I often use right-side if statements, but when the line is long, I think the conciseness and ease-of-reading of the statement is lost. ie. it's harder to tell at a glance what the control flow is. I'd rewrite this into a proper structured block:

return $self->new( %{$data->{Invoices}[0]} ) if ( ref($data->{Invoices +}) eq 'ARRAY' and scalar(@{$data->{Invoices}})==1 );

return 10 if $x > 5; is an example where it is very easy to see what's happening at a glance.

I'd also change some of the naming conventions. Perl typically uses snake_case for vars. UPPER case typically means a constant, CamelCase represents a module name etc. So for things like @PARAMS, I'd go with the idiomatic @params.

Other than what the other monks have already pointed out, most of the rest I've glanced at looks pretty good.


In reply to Re: Review CCP::Xero aspiring CPAN module by stevieb
in thread Review CCP::Xero aspiring CPAN module by localshop

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.