http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13019 --- Comment #5 from Kyle M Hall <kyle@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.