Bugzilla – Attachment 2119 Details for
Bug 4450
Type of error return causes problems
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Proposed Patch
0001-Bug-4450-Use-more-consistent-error-returns-in-C4-Cre.patch (text/plain), 16.91 KB, created by
Chris Cormack
on 2010-05-05 12:43:00 UTC
(
hide
)
Description:
Proposed Patch
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2010-05-05 12:43:00 UTC
Size:
16.91 KB
patch
obsolete
>From 441cc3e5cf5b1c3c31e699505da916778c251c43 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Wed, 5 May 2010 13:37:09 +0100 >Subject: [PATCH] Bug 4450 Use more consistent error returns in C4/Creators/* >Content-Type: text/plain; charset="utf-8" > >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). >--- > 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.6.6.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 4450
:
2119