[Koha-devel] Proposed coding standard: always check for object existence before use

[Jonathan Druart]

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*
<https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20661> - Implement
blocking errors for circulation scripts
*Bug 20351*
<https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=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 at 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 at 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/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20180613/a927a061/attachment.html>