Using a Dictionary to retrieve BOOL values

Question

I have some values pulled from a web service giving me a bool value for a key. I did it this way, but feel like it is repeating without need. How can I refactor this?

Please note that the key on the dictionary has the same name as the object property.

for (NSDictionary *itemDicto in switchesArray) {
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindBookMeeting"]) {
 currentAppswitches.FindBookMeeting = [[itemDicto objectForKey:@"value"]boolValue];
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindCapabilityLocate"]) {
 currentAppswitches.FindCapabilityLocate = [[itemDicto objectForKey:@"value"]boolValue];
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindDesk"]) {
 currentAppswitches.FindDesk = [[itemDicto objectForKey:@"value"]boolValue];
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindEmployeeLocate"]) {
 currentAppswitches.FindEmployeeLocate = [[itemDicto objectForKey:@"value"]boolValue];
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"Help"]) {
 currentAppswitches.Help = [[itemDicto objectForKey:@"value"]boolValue];
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"ReportProblemIT"]) {
 currentAppswitches.ReportProblemIT = [[itemDicto objectForKey:@"value"]boolValue];
 //test ok
 //currentAppswitches.ReportProblemIT = NO;
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"ReportProblemProperty"]) {
 currentAppswitches.ReportProblemProperty = [[itemDicto objectForKey:@"value"]boolValue];
 }
 if ([[itemDicto objectForKey:@"id"]isEqualToString:@"Settings"]) {
 currentAppswitches.Settings = [[itemDicto objectForKey:@"value"]boolValue];
 }
}

Answer 1

Assuming the values under the id keys always match properties of currentAppswitches.

If you can change currentAppswitches's class to accept an NSNumber object rather than a BOOL you have some convenient options.

If currentAppswitches is KVO compliant you could write:

for (NSDictionary *item in switchesArray) {
 id value = [item objectForKey:@"value"];
 id key = [item objectForKey:@"id"];
 [currentAppswitches setValue:value forKeyPath:key];
}

Alternately you could use:

for (NSDictionary *item in switchesArray) {
 BOOL value = [[item objectForKey:@"value"] boolValue];
 //construct a selector for the property setter
 NSMutableString *selectorName = [item objectForKey:@"id"];
 NSString *firstCharacter = [selectorName substringToIndex:1];
 [selectorName stringByReplacingCharactersInRange:NSMakeRange(0,1) withString:[firstCharacter uppercaseString]];
 [selectorName insertString:@"set" atIndex:0];
 [selectorName appendString:@":"];
 SEL selector = NSSelectorFromString(selectorName);
 [currentAppswitches performSelector:selector withObject:value];
}

If currentAppswitches must continue to accept BOOLs then you could create an invocation:

for (NSDictionary *item in switchesArray) {
 id value = [item objectForKey:@"value"];
 //construct a selector for the property setter
 NSMutableString *selectorName = [item objectForKey:@"id"];
 NSString *firstCharacter = [selectorName substringToIndex:1];
 [selectorName stringByReplacingCharactersInRange:NSMakeRange(0,1) withString:[firstCharacter uppercaseString]];
 [selectorName insertString:@"set" atIndex:0];
 [selectorName appendString:@":"];
 SEL selector = NSSelectorFromString(selectorName);
 NSMethodSignature* signature = [[currentAppswitches class] instanceMethodSignatureForSelector:selector];
 NSInvocation* invocation = [NSInvocation invocationWithMethodSignature:signature];
 [invocation setTarget:currentAppswitches];
 [invocation setSelector:selector ];
 [invocation setArgument:&myBoolValue atIndex:2];
 [invocation invoke];
}

• nhgrif: I upvoted this for the first solution proposed. I don't particularly like the idea of building a string to then create a selector. It shouldn't be a problem to use objects in place of bools, considering we can just do @NO/@YES. Or, given BOOL value;, we can @(value).
• Jonah: @nhgrif I agree, building a property setter selector like that is a mess but it was fun to think about how to manage it. All of these are actually potentially fragile and could result in "unrecognized selector" or similar exceptions if the initial assumption that the "ids" match selectors does not hold. I think the first is the only one I'd be happy to see in a production app.
• nhgrif: By the way, if ([currentAppswitches respondsToSelector:selector]) probably belongs in this answer...
• nhgrif: What does [[currentAppswitches class] instanceMethodSignatureForSelector:selector]; return if currentAppswitches doesn't know the selector? nil or throw exception?

Answer 2

First of all, notice that you repeatedly call [itemDicto objectForKey:@"id"] and [[itemDicto objectForKey:@"value"] boolValue]. Let's pull those out and store it as a NSString* and BOOL, respectively.

Also, let's change every if after the first one into an else if to speed up execution via short-circuiting. That means that if the first one is true, we don't want to check every other condition as well (because they will obviously be false), so we skip right to the end.

In more detail:

if ([string isEqualToString:@"A"]) {
 // Always executes with string = @"A", @"B" or @"C"
}
if ([string isEqualToString:@"B"]) {
 // Always executes with string = @"A", @"B" or @"C"
}
if ([string isEqualToString:@"C"]) {
 // Always executes with string = @"A", @"B" or @"C"
}

Whereas with else if:

if ([string isEqualToString:@"A"]) {
 // Always executes with string = @"A", @"B" or @"C"
}
else if ([string isEqualToString:@"B"]) {
 // Only executes with string = @"B" or @"C"
}
else if ([string isEqualToString:@"C"]) {
 // Only executes with string = @"C"
}

Finally, let's change itemDicto to a more reasonable and shorter itemDict.

The resulting code is then:

for (NSDictionary *itemDict in switchesArray) {
 NSString* task = [itemDict objectForKey:@"id"]; // Choose a name that describes the string
 BOOL value = [[itemDict objectForKey:@"value"] boolValue];
 if ([task isEqualToString:@"FindBookMeeting"]) {
 currentAppswitches.FindBookMeeting = value;
 }
 else if ([task isEqualToString:@"FindCapabilityLocate"]) {
 currentAppswitches.FindCapabilityLocate = value;
 }
 else if ([task isEqualToString:@"FindDesk"]) {
 currentAppswitches.FindDesk = value;
 }
 else if ([task isEqualToString:@"FindEmployeeLocate"]) {
 currentAppswitches.FindEmployeeLocate = value;
 }
 else if ([task isEqualToString:@"Help"]) {
 currentAppswitches.Help = value;
 }
 else if ([task isEqualToString:@"ReportProblemIT"]) {
 currentAppswitches.ReportProblemIT = value;
 }
 else if ([task isEqualToString:@"ReportProblemProperty"]) {
 currentAppswitches.ReportProblemProperty = value;
 }
 else if ([task isEqualToString:@"Settings"]) {
 currentAppswitches.Settings = value;
 }
}

Finally, I would suggest have currentAppswitches have a NSMutableDictionary property called tasks where the keys are @"FindBookMeeting", @"FindDesk" etc. and the values are the BOOL's associated with them, but represented as NSNumber. Then, this snippet can be reduced tremendously to this:

for (NSDictionary *itemDict in switchesArray) {
 NSString* task = [itemDict objectForKey:@"id"]; // Choose a name that describes the string
 BOOL value = [[itemDict objectForKey:@"value"] boolValue];
 if (task != nil) {
 [currentAppswitches.tasks setObject:[NSNumber numberWithBool:value] forKey:task];
 }

Answer 3

Since the other answers are quite good, I will just talk about a couple of minor points.

Your variable names need some help. For example:

for (NSDictionary *itemDicto in switchesArray)

When you have variable names that contain an explanation of how that information is being stored (such as Dicto or Array), then if you ever change the method that you use to store the information (such as switching an array to a dictionary or converting the whole structure to a class) you will need to change the names of the variables to match. It is better to avoid this altogether and use a name that does not include the type of information store.

So instead of itemDicto, I would call this something that better describes the information actually contained inside it. Something that makes it more clear what the items are, for example. The same thing is true of switchesArray, it is unclear what the actual meaning of the word switches is in this context.

Good names are very hard to come up with when programming, but they are important. A big part of making your code self documenting is using the best and most explicit names that you can.

I believe this principle extends to the following names as well: FindBookMeeting, FindCapabilityLocate, Help. The names are not very helpful for determining what is going to happen when these choices are selected.

Maybe it is because I just do not completely understand the context of the code, but I have a hard time imagining what the program will do with the BOOL value being returned based on the names you have chosen. I believe that this is a sign that the whole logic structure should be changed to something more clear, but the other answers have provided tips on this topic. Briefly I will suggest that the fact that you might do a Find, make a Report, or look up Settings all from this same menu suggests that too many different things are all being scrunched together in this dictionary.

Another suggestion I would make is to use NSString constants instead of string literals when you are making calls to a dicitonary. Either at the top of the file, or in another file that you import into this file, you would do something like this:

NSString* const kHelp = @"Help";
NSString* const kSettings = @"Settings";
NSString* const kReportProblemIT = @"ReportProblemIT";

This gives you some advantages. First, it is harder to make a mistake later if you ever need to use the string again. If you just type a string literal with @"" then the compiler will not do any error checking. If there are any typos, the code will fail and you might see strange errors that are hard to debug. Second, once you have it set up like the above, when you start typing one of the string constants you will be able to use autocomplete to avoid having to type the whole thing every time. Finally, if you ever want to make changes to the names of the strings, they will all be in one place and easily visible.

• nhgrif: Additionally, comparing two strings that are set to a constant string can sometimes more efficient than comparing string literals.

Answer 4

One more approach would be to transform the data to a dictionary, and then use that dictionary with the method setValuesForKeysWithDictionary: to populate the switches object.

For that, the data has to be transformed from:

[
 { "id" : "FindBookMeeting", "value" : @YES },
 { "id" : "FindCapabilityLocate", "value" : @NO },
 { "id" : "Help", "value" : @YES },
 { "id" : "ReportProblemProperty", "value" : @NO },
 ...
]

to this:

{
 "FindBookMeeting": @YES,
 "FindCapabilityLocate": @NO,
 "Help": @YES,
 "ReportProblemProperty": @NO
}

Once this transformation is done, and if your class is KVC compliant for all those keys, then currentAppSwitches object can be populated using:

[currentAppSwitches setValuesForKeysWithDictionary:switchesDictionary];

The transformation from switchesArray to switchesDictionary can be done as:

NSMutableDictionary *switchesDictionary = [NSMutableDictionary dictionary];
for (NSDictionary *switchDict in switchesArray) {
 NSString *name = switchDict[@"id"];
 id value = switchDict[@"value"];
 switchesDictionary[name] = value;
}

I would change the name to remove the word switch throughout. It appears that these values are user settings, or configuration options that can be ON or OFF. However, since a switch implies the ability to turn something on or off, but the data you're storing is just a set of values that were derived after playing with those switches, I'd name it UserSettings or ApplicationSettings, or ApplicationOptions or something of that nature.

If the format of the incoming dictionary is fixed, you could move the conversion logic into the class itself to have fewer moving parts, and make the logic more or less self contained. I'd imagine the interface would look something like:

@interface ApplicationOptions
@property (assign) BOOL FindBookMeeting;
@property (assign) BOOL Help;
...
- (instancetype)initWithOptions:(NSArray *)options;
@end
@implementation
- (instancetype)initWithOptions:(NSArray *)options {
 self = [super init];
 if (self) {
 NSMutableDictionary *optionsDict = [NSMutableDictionary dictionary];
 for (NSDictionary *option in options) {
 NSString *name = option[@"id"];
 id value = option[@"value"];
 optionsDict[name] = value;
 }
 [self setValuesForKeysWithDictionary:optionsDict];
 }
 return self;
}
@end