[Koha-bugs] [Bug 13019] Add base classes on which to build Koha objects

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

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13019

--- Comment #5 from Kyle M Hall <kyle at bywatersolutions.com> ---
(In reply to Jonathan Druart from comment #4)
> I tried to make Koha::Borrower extends Koha::Schema::Result::Borrower, but I
> get an error.
> The idea was to remove the AUTOLOAD and avoid the IsStored, IsChanged, Id,
> etc. methods.
> 
> [the error was: Unable to perform storage-dependent operations with a
> detached result source (source '_unnamed_' is not associated with a schema)]
> I suppose I tried to do something very bad :)
> 
> So, some additional remarks on the patch:

> - "result" and "columns" should be _result and _columns to avoid collisions
> later, don't you think?

Good idea

> - Are you sure it is useful to test if the row is stored and has been
> changed (Store method)? Not sure about the perf, maybe DBIC does that (?).

Yeah, I'd have to check the DBIC code, but that seems entirely likely.

> - IsStored should be InStorage or completely removed, if useless.

Again, your probably right. I can't see a use for this right now and we can
always add an InStorage method at a later date if it *is* needed for something.

> - Delete could call delete in an eval, to avoid a call to in_storage

Do you think that will improve efficiency? in_storage has already been
calculated and is just a dbic property. It seems like using eval would take
more time.

> - what about 2 constructors? new and new_from_rs? The ResultSet should not
> be create if not used later

For Koha::Object? That's a good idea.

> - what about a _result method? It would return ->{_result} if already
> exists, otherwise create it. All occurrences to ->{result} will be replaced
> by ->_result

Also a good idea.

I'll see if I can't make some time next week to implement these changes.

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