[Koha-bugs] [Bug 26555] Add a way for Koha::Object(s) to carry execution information

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Thu Oct 8 15:03:00 CEST 2020


https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26555

--- Comment #17 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Jonathan Druart from comment #16)
> Tomas, I am not a fan of this, and I am wondering if the following approach
> would not be better.

I need to explain that this was just my attempt to find a more elegant way of
doing this:

https://gitlab.com/koha-community/Koha/-/blob/master/Koha/Item.pm#L886

i.e. passing execution information to the caller, while keeping chainability on
the methods.

That was the only example I found in which we did something similar-ish. I
renamed them from Koha::Object::Error to Koha::Object::Message as well,  for
that reason.

> Imagine you have one or more messages (ie. not blocking errors) to pass to
> the caller, then you could throw a Koha::Exception::Messages (or
> Koha::Exception::Errors?) that would contain a set of Koha::Exception.
> That would allow us to tell the callers several things happened, and let it
> handle the situation how it wants.
> 
> What do you think?

I prefer exceptions as well. But how would that look in the code?

My implementation means you need to catch error conditions in your sub, and add
error messages to notify. Then you just return normally. And is up to the
caller to do something with those errors/messages. basket.pl and closeorder.pl
don't care much about them, they will, but I intended not to change behaviour
yet.

With your proposal, you would need to accumulate the messages (maybe reusing
this ->add_message method) and at the end of the method, check if there are
messages, and throw an exception. It is not mutually exclusive with this
implementation, actually. It is just a matter of how you want to do/use it.

Take the basket.pl example. It doesn't bother to notify errors (I'm not
implying that's correct, but you could want that). Do you really want to
enforce a try/catch block for catching the error messages? And what if there
are other exceptions?


try {
    $object->the_method;
}
catch {
   if ( ref($->isa) eq 'Koha::Object::Messages' ) {
      $check_errors = 1;
   }
   elsif ( ref($_) eq 'Koha::Exception::XXX' ) {
      HANDLE_ERROR();
   }
   else {
      UNHANDLED_EXCEPTION();
   }
};

NORMAL_EXECUTION({ check_errors => $check_errors })

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list