From: Colin Campbell <colin.campb...@ptfs-europe.com>
A lot of routines were defaulting to return -1 in error conditions
but calling code was expecting a ref or object
use return with explicit undef (or emptyness in array context)
for these cases. Extended this to cases where return was not tested
( -1 might in some cases be legit data).
Signed-off-by: Chris Nighswonger <cnighswon...@foundations.edu>
---
C4/Creators/Batch.pm | 10 ++++++----
C4/Creators/Layout.pm | 18 ++++++++----------
C4/Creators/Lib.pm | 27 +++++++++++++++------------
C4/Creators/PDF.pm | 8 ++++----
C4/Creators/Profile.pm | 19 ++++++++++---------
C4/Creators/Template.pm | 22 ++++++++++------------
6 files changed, 53 insertions(+), 51 deletions(-)
diff --git a/C4/Creators/Batch.pm b/C4/Creators/Batch.pm
index cff7419..fabfdd7 100644
--- a/C4/Creators/Batch.pm
+++ b/C4/Creators/Batch.pm
@@ -152,10 +152,12 @@ sub retrieve {
push (@{$self->{'items'}}, {$number_type => $record->{$number_type},
label_id => $record->{'label_id'}});
$record_flag = 1; # true if one or more rows were retrieved
}
- return -2 if $record_flag == 0; # a hackish sort of way of indicating
no such record exists
+ if ($record_flag == 0) {
+ return; # no such record exists return undef
+ }
if ($sth->err) {
warn sprintf('Database returned the following error on attempted
SELECT: %s', $sth->errstr);
- return -1;
+ return;
}
$self->{'batch_stat'} = 1;
bless ($self, $type);
@@ -263,12 +265,12 @@ This module provides methods for creating, and otherwise
manipulating batch obje
=head2 C4::Labels::Batch->retrieve(batch_id => $batch_id)
- Invoking the I<retrieve> method constructs a new batch object containing
the current values for batch_id. The method returns a new object upon success
and 1 upon failure.
+ Invoking the I<retrieve> method constructs a new batch object containing
the current values for batch_id. The method returns a new object upon success
and undefined upon failure.
Errors are logged to the Apache log.
examples:
- my $batch = C4::Labels::Batch->retrieve(batch_id => 1); # Retrieves
batch 1 and returns an object containing the record
+ my $batch = C4::Labels::Batch->(batch_id => 1); # Retrieves batch 1
and returns an object containing the record
=head2 delete()
diff --git a/C4/Creators/Layout.pm b/C4/Creators/Layout.pm
index a38f347..d5c41ea 100644
--- a/C4/Creators/Layout.pm
+++ b/C4/Creators/Layout.pm
@@ -63,7 +63,7 @@ sub new {
my $invocant = shift;
my $self = '';
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my $type = ref($invocant) || $invocant;
if (grep {$_ eq 'Labels'} @_) {
@@ -101,7 +101,7 @@ sub retrieve {
$sth->execute($opts{'layout_id'}, $opts{'creator'});
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
my $self = $sth->fetchrow_hashref;
bless ($self, $type);
@@ -136,6 +136,7 @@ sub delete {
warn sprintf('Database returned the following error on attempted
DELETE: %s', $sth->errstr);
return -1;
}
+ return; # return no error code on success
}
sub save {
@@ -156,7 +157,7 @@ sub save {
$sth->execute(@params);
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
return $self->{'layout_id'};
}
@@ -178,7 +179,7 @@ sub save {
$sth->execute(@params);
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
my $sth1 = C4::Context->dbh->prepare("SELECT MAX(layout_id) FROM
creator_layouts;");
$sth1->execute();
@@ -191,22 +192,19 @@ sub save {
sub get_attr {
my $self = shift;
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my ($attr) = @_;
if (exists($self->{$attr})) {
return $self->{$attr};
}
- else {
- return -1;
- }
return;
}
sub set_attr {
my $self = shift;
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my %attrs = @_;
foreach my $attrib (keys(%attrs)) {
@@ -389,7 +387,7 @@ R = Right
=head2 get_attr($attribute)
- Invoking the I<get_attr> method will return the value of the requested
attribute or -1 on errors.
+ Invoking the I<get_attr> method will return the value of the requested
attribute or undef on error.
example:
C<my $value = $layout->get_attr($attribute);>
diff --git a/C4/Creators/Lib.pm b/C4/Creators/Lib.pm
index 6e07847..d3199f6 100644
--- a/C4/Creators/Lib.pm
+++ b/C4/Creators/Lib.pm
@@ -135,7 +135,7 @@ my $output_formats = [
=head2 C4::Creators::Lib::get_all_templates()
- This function returns a reference to a hash containing all templates upon
success and 1 upon failure. Errors are logged to the Apache log.
+ This function returns a reference to a hash containing all templates upon
success and undefined upon failure. Errors are logged to the Apache log.
examples:
@@ -152,7 +152,7 @@ sub get_all_templates {
$sth->execute();
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
ADD_TEMPLATES:
while (my $template = $sth->fetchrow_hashref) {
@@ -163,7 +163,7 @@ sub get_all_templates {
=head2 C4::Creators::Lib::get_all_layouts()
- This function returns a reference to a hash containing all layouts upon
success and 1 upon failure. Errors are logged to the Apache log.
+ This function returns a reference to a hash containing all layouts upon
success and undefined upon failure. Errors are logged to the Apache log.
examples:
@@ -180,7 +180,7 @@ sub get_all_layouts {
$sth->execute();
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
ADD_LAYOUTS:
while (my $layout = $sth->fetchrow_hashref) {
@@ -191,7 +191,7 @@ sub get_all_layouts {
=head2 C4::Creators::Lib::get_all_profiles()
- This function returns an arrayref whose elements are hashes containing all
profiles upon success and 1 upon failure. Errors are logged
+ This function returns an arrayref whose elements are hashes containing all
profiles upon success and undefined upon failure. Errors are logged
to the Apache log. Two parameters are accepted. The first limits the
field(s) returned. This parameter should be string of comma separted
fields. ie. "field_1, field_2, ...field_n" The second limits the records
returned based on a string containing a valud SQL 'WHERE' filter.
@@ -214,7 +214,7 @@ sub get_all_profiles {
$sth->execute();
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
ADD_PROFILES:
while (my $profile = $sth->fetchrow_hashref) {
@@ -235,7 +235,7 @@ sub get_all_image_names {
$sth->execute();
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
grep {push @$image_names, {type => $$_[0], name => $$_[0], selected => 0}}
@{$sth->fetchall_arrayref([0])};
return $image_names;
@@ -266,7 +266,7 @@ sub get_batch_summary {
$sth->execute($params{'creator'});
if ($sth->err) {
warn sprintf('Database returned the following error on attempted
SELECT: %s', $sth->errstr);
- return -1;
+ return;
}
ADD_BATCHES:
while (my $batch = $sth->fetchrow_hashref) {
@@ -275,7 +275,7 @@ sub get_batch_summary {
$sth1->execute($batch->{'batch_id'}, $params{'creator'});
if ($sth1->err) {
warn sprintf('Database returned the following error on attempted
SELECT count: %s', $sth1->errstr);
- return -1;
+ return;
}
my $count = $sth1->fetchrow_arrayref;
$batch->{'_item_count'} = @$count[0];
@@ -315,7 +315,7 @@ sub get_label_summary {
$sth->execute($item->{'item_number'}, $params{'batch_id'});
if ($sth->err) {
warn sprintf('Database returned the following error on attempted
SELECT: %s', $sth->errstr);
- return -1;
+ return;
}
my $record = $sth->fetchrow_hashref;
my $label_summary;
@@ -361,7 +361,7 @@ sub get_card_summary {
$sth->execute($item->{'borrower_number'});
if ($sth->err) {
warn sprintf('Database returned the following error on attempted
SELECT: %s', $sth->errstr);
- return -1;
+ return;
}
my $record = $sth->fetchrow_hashref;
my $card_summary->{'_card_number'} = $card_number;
@@ -535,7 +535,10 @@ sub get_table_names {
sub html_table {
my $headers = shift;
my $data = shift;
- return undef if scalar(@$data) == 0; # no need to generate a table if
there is not data to display
+ # no need to generate a table if there is not data to display
+ unless ( @{$data} ) {
+ return;
+ }
my $table = [];
my $fields = [];
my @table_columns = ();
diff --git a/C4/Creators/PDF.pm b/C4/Creators/PDF.pm
index d181bdb..e99e6e8 100644
--- a/C4/Creators/PDF.pm
+++ b/C4/Creators/PDF.pm
@@ -152,8 +152,8 @@ sub Jpeg {
sub prAltJpeg
{ my ($iData, $iWidth, $iHeight, $iFormat,$aiData, $aiWidth, $aiHeight,
$aiFormat) = @_;
my ($namnet, $utrad);
- if (! $PDF::Reuse::pos) # If no output is active, it is
no use to continue
- { return undef;
+ if (! $PDF::Reuse::pos) { # If no output is active, it is
no use to continue
+ return;
}
prJpegBlob($aiData, $aiWidth, $aiHeight, $aiFormat);
my $altObjNr = $PDF::Reuse::objNr;
@@ -182,8 +182,8 @@ sub prAltJpeg
sub prJpegBlob
{ my ($iData, $iWidth, $iHeight, $iFormat, $altArrayObjNr) = @_;
my ($iLangd, $namnet, $utrad);
- if (! $PDF::Reuse::pos) # If no output is active, it is
no use to continue
- { return undef;
+ if (! $PDF::Reuse::pos) { # If no output is active, it is
no use to continue
+ return;
}
my $checkidOld = $PDF::Reuse::checkId;
if (!$iFormat)
diff --git a/C4/Creators/Profile.pm b/C4/Creators/Profile.pm
index aab0e27..3c1a403 100644
--- a/C4/Creators/Profile.pm
+++ b/C4/Creators/Profile.pm
@@ -57,8 +57,8 @@ sub _conv_points {
sub new {
my $invocant = shift;
- if (_check_params(@_) eq 1) {
- return -1;
+ if (_check_params(@_) == 1) {
+ return;
}
my $type = ref($invocant) || $invocant;
my $self = {
@@ -85,7 +85,7 @@ sub retrieve {
$sth->execute($opts{'profile_id'}, $opts{'creator'});
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
my $self = $sth->fetchrow_hashref;
$self = _conv_points($self) if ($opts{convert} && $opts{convert} == 1);
@@ -122,6 +122,7 @@ sub delete {
warn sprintf('Database returned the following error on attempted
DELETE: %s', $sth->errstr);
return -1;
}
+ return;
}
sub save {
@@ -142,7 +143,7 @@ sub save {
$sth->execute(@params);
if ($sth->err) {
warn sprintf('Database returned the following error on attempted
UPDATE: %s', $sth->errstr);
- return -1;
+ return;
}
return $self->{'profile_id'};
}
@@ -164,7 +165,7 @@ sub save {
$sth->execute(@params);
if ($sth->err) {
warn sprintf('Database returned the following error on attempted
INSERT: %s', $sth->errstr);
- return -1;
+ return;
}
my $sth1 = C4::Context->dbh->prepare("SELECT MAX(profile_id) FROM
printers_profile;");
$sth1->execute();
@@ -176,7 +177,7 @@ sub save {
sub get_attr {
my $self = shift;
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my ($attr) = @_;
if (exists($self->{$attr})) {
@@ -184,7 +185,7 @@ sub get_attr {
}
else {
warn sprintf('%s is currently undefined.', $attr);
- return -1;
+ return;
}
}
@@ -278,14 +279,14 @@ CM = SI Centimeters (28.3464567 points per)
=head2 save()
Invoking the I<save> method attempts to insert the profile into the
database if the profile is new and update the existing profile record if the
profile exists. The method returns
- the new record profile_id upon success and -1 upon failure (This avoids
conflicting with a record profile_id of 1). Errors are logged to the Apache log.
+ the new record profile_id upon success and undef upon failure (This avoids
conflicting with a record profile_id of 1). Errors are logged to the Apache log.
example:
C<my $exitstat = $profile->save(); # to save the record behind the
$profile object>
=head2 get_attr($attribute)
- Invoking the I<get_attr> method will return the value of the requested
attribute or -1 on errors.
+ Invoking the I<get_attr> method will return the value of the requested
attribute or undef on errors.
example:
C<my $value = $profile->get_attr($attribute);>
diff --git a/C4/Creators/Template.pm b/C4/Creators/Template.pm
index 0bbeadf..e43a3cd 100644
--- a/C4/Creators/Template.pm
+++ b/C4/Creators/Template.pm
@@ -89,7 +89,7 @@ sub new {
my $invocant = shift;
my $type = ref($invocant) || $invocant;
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my $self = {
profile_id => 0,
@@ -124,7 +124,7 @@ sub retrieve {
$sth->execute($opts{'template_id'}, $opts{'creator'});
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
my $self = $sth->fetchrow_hashref;
$self = _conv_points($self) if (($opts{convert} && $opts{convert} == 1) ||
$opts{profile_id});
@@ -152,7 +152,7 @@ sub delete {
if (scalar(@query_params) < 2) { # If there is no template id or creator
type then we cannot delete it
warn sprintf('%s : Cannot delete template as the template id is
invalid or non-existant.', $call_type) if !$query_params[0];
warn sprintf('%s : Cannot delete template as the creator type is
invalid or non-existant.', $call_type) if !$query_params[1];
- return -1;
+ return;
}
my $query = "DELETE FROM " . $opts{'table_name'} . " WHERE template_id = ?
AND creator = ?";
my $sth = C4::Context->dbh->prepare($query);
@@ -178,7 +178,7 @@ sub save {
$sth->execute(@params);
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
$self->{'template_stat'} = 1;
return $self->{'template_id'};
@@ -202,7 +202,7 @@ sub save {
$sth->execute(@params);
if ($sth->err) {
warn sprintf('Database returned the following error: %s',
$sth->errstr);
- return -1;
+ return;
}
my $sth1 = C4::Context->dbh->prepare("SELECT MAX(template_id) FROM " .
$opts{'table_name'} . ";");
$sth1->execute();
@@ -216,21 +216,19 @@ sub save {
sub get_attr {
my $self = shift;
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my ($attr) = @_;
if (exists($self->{$attr})) {
return $self->{$attr};
}
- else {
- return -1;
- }
+ return;
}
sub set_attr {
my $self = shift;
if (_check_params(@_) eq 1) {
- return -1;
+ return;
}
my %attrs = @_;
foreach my $attrib (keys(%attrs)) {
@@ -374,7 +372,7 @@ CM = SI Centimeters (28.3464567 points per)
=head2 save()
Invoking the I<save> method attempts to insert the template into the
database if the template is new and update the existing template record if
- the template exists. The method returns the new record template_id upon
success and -1 upon failure (This avoids template_ids conflicting with a
+ the template exists. The method returns the new record template_id upon
success and undef upon failure (This avoids template_ids conflicting with a
record template_id of 1). Errors are logged to the Apache log.
example:
@@ -382,7 +380,7 @@ CM = SI Centimeters (28.3464567 points per)
=head2 get_attr($attribute)
- Invoking the I<get_attr> method will return the value of the requested
attribute or -1 on errors.
+ Invoking the I<get_attr> method will return the value of the requested
attribute or undef on errors.
example:
C<my $value = $template->get_attr($attribute);>
--
1.7.0.4
_______________________________________________
Koha-patches mailing list
Koha-patches@lists.koha.org
http://lists.koha.org/mailman/listinfo/koha-patches
