NSFetchedResultsController convenience methods should not call performFetch

[duanefields]

Currently the NSFetchedResultsController methods (i.e. "fetchAllSortedBy") call performFetch before returning. However, this means you can't actually set a delegate (the arguably most common use case) or tweak the fetch request before executing your query. Yes, can just do the perform fetch again, but that doubles up the query time. Here's a typical use case that illustrates how I would like to call it...

    _fetchedResultsController = [[CTDiaryEntry fetchAllSortedBy:@"category,entryDate" ascending:YES withPredicate:predicate groupBy:@"category"] retain];
    _fetchedResultsController.fetchRequest.includesSubentities = YES;
    _fetchedResultsController.fetchRequest.includesPendingChanges = NO;
    _fetchedResultsController.delegate = self;

    [_fetchedResultsController performFetch:nil]

Since the helper returns a fetchedResultsController, rather than objects, it seems appropriate to simply construct the results controller and not to "act" on it. that's the caller's job.

[magicalpanda]

The goal of this helper method is to remove the boilerplate code for you. I have a couple of thoughts for solutions:

1) make another method that has the delegate as a parameter 2) have a global or method level option to perform the fetch before the method has completed.

Which do you think would work best? I supposed a third option is to remove the performFetch call... :)

[duanefields]

I hear you. It seems like the helper here "help me create my fetchedResultsController", but I also agree that removing the performFetch would break people. Adding a version of the call that takes a delegate feels appropriate, but doesn't solve the problem per say - you still have all the other options to set, and then it would have this funky rule that perform fetch doesn't get called for some flavors of "fetchAll*", which would be inconsistent.

The global behavior change doesn't feel right either, unless it were more of a "backwards compatibility flag", where starting with the next version these methods do not call performFetch unless you have #define VERSION_X_COMPATIBLE or #define MAGICAL_RECORD_CALLS_FETCH or some such.

[magicalpanda]

One example I've started to borrow from Ruby is the idea of an options dictionary. Now, Ruby makes dictionaries syntactically cheap with form like { :key => "value" }, but the idea is still pretty interesting. You can just set an option saying you want to auto fetch or not, and it goes…This may take some trial and error to get the method name and parameters right...

Saul Mora Founding Panda, Software Journeyman saul@magicalpanda.com (mailto:saul@magicalpanda.com)

On Tuesday, September 6, 2011 at 12:53 PM, duanefields wrote:

I hear you. It seems like the helper here "help me create my fetchedResultsController", but I also agree that removing the performFetch would break people. Adding a version of the call that takes a delegate feels appropriate, but doesn't solve the problem per say - you still have all the other options to set, and then it would have this funky rule that perform fetch doesn't get called for some flavors of "fetchAll*", which would be inconsistent.

The global behavior change doesn't feel right either, unless it were more of a "backwards compatibility flag", where starting with the next version these methods do not call performFetch unless you have #define VERSION_X_COMPATIBLE or #define MAGICAL_RECORD_CALLS_FETCH or some such.

Reply to this email directly or view it on GitHub: https://github.com/magicalpanda/MagicalRecord/issues/65#issuecomment-2020906

[duanefields]

Yea, this is typically done in Objective C with enums and masks, so you can combine as many as you'd like. Not sure that remembering the 3 magic options is actually any easier/better than simply post configuring the boolean properties though, since you're just coming up with flags the directly mirror existing properties on the object you are returning.

On Sep 6, 2011, at 2:19 PM, magicalpanda wrote:

One example I've started to borrow from Ruby is the idea of an options dictionary. Now, Ruby makes dictionaries syntactically cheap with form like { :key => "value" }, but the idea is still pretty interesting. You can just set an option saying you want to auto fetch or not, and it goesThis may take some trial and error to get the method name and parameters right...

Saul Mora Founding Panda, Software Journeyman saul@magicalpanda.com (mailto:saul@magicalpanda.com)

On Tuesday, September 6, 2011 at 12:53 PM, duanefields wrote:

I hear you. It seems like the helper here "help me create my fetchedResultsController", but I also agree that removing the performFetch would break people. Adding a version of the call that takes a delegate feels appropriate, but doesn't solve the problem per say - you still have all the other options to set, and then it would have this funky rule that perform fetch doesn't get called for some flavors of "fetchAll*", which would be inconsistent.

The global behavior change doesn't feel right either, unless it were more of a "backwards compatibility flag", where starting with the next version these methods do not call performFetch unless you have #define VERSION_X_COMPATIBLE or #define MAGICAL_RECORD_CALLS_FETCH or some such.

Reply to this email directly or view it on GitHub: https://github.com/magicalpanda/MagicalRecord/issues/65#issuecomment-2020906

Reply to this email directly or view it on GitHub: https://github.com/magicalpanda/MagicalRecord/issues/65#issuecomment-2021198

Duane Fields duane@duanefields.com

[casademora]

I think what I may end up doing for this issue is to have a sort of global setting in the MagicalRecordHelpers object that will change toggle this feature on and off, with the default being perform the fetch before returning....

There are some "global" defaults there now for things like "shouldAutoSetDefaultStore", etc. How do those sound to you?

[duanefields]

But it seems like this is a case by case option, not a global default. If the default is to perform the fetch, then I encounter a case where I need to set a delegate and so forth, I then have to go back and change all of my other values.

What about adding one more parameter to the fetchAllSortedBy and similar (to the full signature only, not all the easy helper) performFetch:BOOL

That way if you need to disable performFetch you can, just use the LONG from of fetchAllSortedBy ( and similar) and specify NO

[magicalpanda]

I've added a new delegate parameter to a couple of the fetchAll methods. Can you give them a try and let me know if they work for you. This should also remove the compile time feature of enabling the file cache. It's now a runtime parameter also.

[duanefields]

Ok, thanks.

Duane Fields duane@duanefields.com

On Nov 15, 2011, at 2:18 PM, Magical Panda Software wrote:

I've added a new delegate parameter to a couple of the fetchAll methods. Can you give them a try and let me know if they work for you. This should also remove the compile time feature of enabling the file cache. It's now a runtime parameter also.

---

Reply to this email directly or view it on GitHub: https://github.com/magicalpanda/MagicalRecord/issues/65#issuecomment-2750606