https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #8)
I am certainly missing the context here, but this feels like unnecessary overhead.
I thought the examples in IdP and BackgroundJob were enough context. But fair.
You are basically adding a Koha::Object base class for JSON decode and encode calls. That's all.
We are repeating code everywhere, and I provide a way to extend Koha::Object-derived classes with accessors for JSON fields. Yes. It sounds like a simple enough approach. If we did a regular class, we would need to instantiate it as needed and it feels counter productive and unnecessary. (In reply to Martin Renvoize from comment #9)
I'm still contemplating this...
Considering https://metacpan.org/pod/DBIx::Class::InflateColumn::Serializer::JSON as an example to compare to.
I agree with this approach and I found it interesting. YOu mentioned it after I already wrote this so I thought I would post this to start the discussion. Will think about it. The main drawback is that devs might be caught by surprise with inflation, but it shouldn't be a problem. It will jus tbe a general behavior change and we would need to measure performance/memory overhead, as my implementation will only introduce overhead if the new accessors are really called. I tried to provide real-life examples on how it would be used to make the point code-wise. (In reply to Jonathan Druart from comment #8)
The exception is there only to raise a Koha exception, but having a JSON exception is also fine. The MissingParameter exceptions are not needed either IMO, you will get a perl error if you are calling the method with missing mandatory parameter.
Yeah, I tried that. You get - "Can't call method undefined", I prefer a named exception but this is really taste. - As of JSON, I think I picked what we already had. But it is fair not to throw our own exception; again, a matter of taste. -- You are receiving this mail because: You are watching all bug changes.