[Koha-bugs] [Bug 32370] Provide a generic set of tools for JSON fields

[bugzilla-daemon at bugs.koha-community.org]

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

--- Comment #10 from Tomás Cohen Arazi <tomascohen at 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.