https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370
--- Comment #10 from Tomás Cohen Arazi <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
