Hi Barton,

Yes, and no. It depends.

Example 1)

my $biblio_to_edit = Koha::Biblios->find( $query->param('biblionumber') );

say sprintf "you are editing %s", $biblio_to_edit->title;

Here we need to display a message if the biblio you want to edit does no longer exist (or invalid id).

Example 2)

sub one_more_sub {

my ( $itemnumber ) = @_;

my $item = Koha::Items->find( $itemnumber );

return unless $item;

say $item->homebranch->branchname;

say Koha::Frameworks->find($item->biblio->frameworkcode);

}

Here we do not need to make sure the homebranch or the biblio or the framework exist. The DB constraints take care of that for us. If the itemnumber exists, other objects exist.

In that case of 1) we should stop the process at that point if the biblio does not exist, no need to continue.

On bug 18403 I have introduced a way to do that, it is as much useful as ugly. We wanted to display a message if 1. the borrowernumber does not exist, or 2. the logged-in patron does not have the permission to view patron's info for a given borrowernumber. It looks like:

my $logged_in_user = Koha::Patrons->find( $loggedinuser )

or die "Not logged in";
output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } );

What does it do? It dies in case $logged_in_user does not exist (it should *never* happen, but... who really knows?)

And will render the template by setting an error flag "unknown_patron" or "cannot_see_patron_infos" if needed (for more info, see C4::Output::output_and_exit_if_error and blocking_errors.inc).

Basically, we should avoid indentation blocks (if ( defined $item ) { # then continue }), it is usually a bad idea.

See also:

Bug 20661 - Implement blocking errors for circulation scripts

Bug 20351 - Implement blocking errors for serials scripts

Help me to make them in, and I will implement more.

Going further, see a previous discussion "Expected behaviour if itemtype does not exist" - http://lists.koha-community.org/pipermail/koha-devel/2017-August/043924.html

Cheers,

Jonathan

On Wed, 13 Jun 2018 at 14:23 Barton Chittenden <barton@bywatersolutions.com> wrote:

Calling a method or accessing a member of an undefined object will throw a fatal error that looks like this: "can't call method * on an undefined value at ..."

For example, this code will cause a software error if $data->{itemnumber} exists, but the item doesn't exist (maybe it's been deleted?):

my $item = Koha::Items->find( $data->{itemnumber} );
my $biblio = $item->biblio;

How this is handled will of course depend on context -- maybe we should carp and continue, or maybe just

my $item = Koha::Items->find( $data->{itemnumber} );
my $biblio = $item->biblio if defined $item;

I would like to add a coding standard that we always check for this.

Thanks,

--Barton
_______________________________________________
Koha-devel mailing list
Koha-devel@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/