2

I have two methods in my objective-c class that do similar but different things. (they both retrieve a set of core-data records containing JSON, unpack and examine the JSON documents and do some stuff depending upon the structure of the JSON).

The first method looks like this:

+(NSDictionary*)getListOfResponsesWithForm:(NSString*)formId
{
    NSError* requestError = nil;

    // NSIndexSets don't allow easy targetted access into the set, so use arrays instead.
    NSMutableArray* indexSetOfAllEntries = [[NSMutableArray alloc] init];
    NSMutableArray* indexSetOfEntriesForLoggedOnUser = [[NSMutableArray alloc] init];

    NSString* activeUserEmail = getActiveUser().email;


    NSFetchRequest* fetchRequest = [ [NSFetchRequest alloc] init];

    NSEntityDescription* entityDesc = [NSEntityDescription entityForName:@"Response" inManagedObjectContext:getApp().managedObjectContext];

    [fetchRequest setEntity:entityDesc];

    // Sort by lastmodifieddatelocal
    NSSortDescriptor *sort = [[NSSortDescriptor alloc] initWithKey:@"lastmodifieddatelocal" ascending:NO];
    [fetchRequest setSortDescriptors:[NSArray arrayWithObject:sort]];

    NSArray* instances = [getApp().managedObjectContext executeFetchRequest:fetchRequest error:&requestError];

    NSMutableArray* responses = [[NSMutableArray alloc] init];

    for (Response* response in instances) {


        NSData *jsonData = [response.json dataUsingEncoding:NSUTF8StringEncoding];
        NSDictionary* dictionary = [HPSJSON getDictionaryFromDataWithoutSuccessTest:jsonData];

        NSString* userEmailFromResponse = [HPSJSON getStringForKey: @"_useremail" inDictionary:dictionary];

        NSString* formIdFromResponse =  [HPSJSON getNestedIdForKey: @"_formid" inDictionary: dictionary];

        if ([formId caseInsensitiveCompare:formIdFromResponse]==NSOrderedSame)
        {
            [responses addObject: response];

            [indexSetOfAllEntries addObject:[NSNumber numberWithInt:responses.count-1]];

            if ([activeUserEmail caseInsensitiveCompare:userEmailFromResponse]==NSOrderedSame)
            {
                [indexSetOfEntriesForLoggedOnUser addObject:[NSNumber numberWithInt:responses.count-1]];
            }

        }

    }

    NSMutableDictionary* results = [[NSMutableDictionary alloc] init];
    [results setObject:responses forKey:@"responses"];
    [results setObject:indexSetOfAllEntries forKey:@"allindexes"];
    [results setObject:indexSetOfEntriesForLoggedOnUser forKey:@"indexesforactiveuser"];

    return results;

}

The second method looks like this:

+(NSInteger)getCountOfResponsesWithForm:(NSString*)formId
{
    NSError* requestError = nil;

    NSString* activeUserEmail = getActiveUser().email;

    NSFetchRequest* fetchRequest = [ [NSFetchRequest alloc] init];

    NSEntityDescription* entityDesc = [NSEntityDescription entityForName:@"Response" inManagedObjectContext:getApp().managedObjectContext];

    [fetchRequest setEntity:entityDesc];

    NSArray* instances = [getApp().managedObjectContext executeFetchRequest:fetchRequest error:&requestError];

    NSInteger countOfResponses=0;

    for (Response* response in instances) {

        NSData *jsonData = [response.json dataUsingEncoding:NSUTF8StringEncoding];
        NSDictionary* dictionary = [HPSJSON getDictionaryFromDataWithoutSuccessTest:jsonData];

        NSString* userEmailFromResponse = [HPSJSON getStringForKey: @"_useremail" inDictionary:dictionary];

        NSString* formIdFromResponse =  [HPSJSON getNestedIdForKey: @"_formid" inDictionary: dictionary];

        if ([formId caseInsensitiveCompare:formIdFromResponse]==NSOrderedSame)
        {
            if ([activeUserEmail caseInsensitiveCompare:userEmailFromResponse]==NSOrderedSame)
            {
                countOfResponses++;
            }

        }

    }

    return countOfResponses;

}

There is quite a lot of duplicate code here, and I feel I am abusing DRY to some extent. However, attempting to combine the methods into one method will introduce some complication that will somewhat obfuscate what each individual method does.

Can anyone offer advice on the most elegant way to implement the functionality that is encompassed on both methods? Stay with the two methods? Create one more complex method sprinkled with conditions? Break out into a different class?

Is their a relevant design pattern for this scenario? Thanks.

Journeyman
  • 10,011
  • 16
  • 81
  • 129

4 Answers4

1

I would suggest:

  1. identify common blocks (e.g., NSFetchRequest* fetchRequest..., NSEntityDescription* entityDesc);

  2. for each common block, define a method that gets the proper arguments and does what is required of them;

  3. call such method to improve on DRY, e.g.:

    <getInstances>
    <for each instance>
       <extract data from instance>
       <do your processing on data>
    

I would not think in terms of design patterns, rather in terms of refactoring in this case. You could of course encapsulate all this in a class of its own, but I do not know the role played by the class presently containing your code, so I cannot say anything more detailed.

BTW, I tend to use design patterns at a higher level of abstraction (that is why they are called design pattern, although many are mostly useful at the architectural level). In this case some refactoring recipes (as you might find googling for them) are better suited I think.

sergio
  • 68,819
  • 11
  • 102
  • 123
1

I would consider Template design pattern.

It basically encapsultes basic behavior in template method inside the base class and conrete behavior is implemented inside underlying child classes that implements (or overrides) abstract methods defined in base class.

Interesting resource also here on SO: Objective-C - Template methods pattern?

Community
  • 1
  • 1
mipe34
  • 5,596
  • 3
  • 26
  • 38
0

I am not good with Xcode, so let me just write some logic instead.

It seems to me that your second method can be simplified by :

int getCountOfResponsesWithForm(String a)
{
    Dictionary dic  = getListOfResponsesWithForm(a);
    return dic.length();
}

You might interested to read Facade Design Pattern.

Rudy
  • 7,008
  • 12
  • 50
  • 85
  • You are absolutely correct. The reason I didn't implement it like that is performance (the counts might be quite high, and it seemed to me that the memory usage might suffer unnecessarily doing it this way). Good point though, thanks. – Journeyman Jan 08 '13 at 10:48
0

Use design pattern Visitor or Command. This lets you decouple fetching and iterating over the responses from what action to take.

What you have here are two things: A preparation + loop + marshalling step and a processing step. One part is the same (the former) and one varies (the latter). So you want to factor out the latter, so you can avoid making several copies of the former.

You can do so either by making the processing step a dummy method that is overloaded in subclasses, or by passing in "something" that can perform the step. This "something" will usually be either a Command or a Visitor.

It all really boils down to whether you want to use inheritance or composition. Personally I tend to prefer composition.

Anders Johansen
  • 10,165
  • 7
  • 35
  • 52