I believe in boy scout rule: 'Always leave the campground cleaner than you found it.', that's why I constantly refactor my code so it's clean and tidy.
What are few less-known refactoring tricks I use to simplify code and make it more readable?
Dictionary mappings
Let's say you have a switch statement that assigns some value:
switch(condition) {
case value1:
result = @"valueFor1";
break;
case value2:
result = @"valueFor2";
break;
case value3:
result = @"valueFor3";
break;
case value4:
result = @"valueFor4";
break;
case value5:
result = @"valueFor5";
break;
default:
result = @"valueForDefault";
break;
}
return result;
Keep in mind this is just switch for 5 values, imagine one for more.
Let's use dictionary mapping to simplify it:
static NSDictionary *mapping = nil;
if(!mapping) {
mapping = @{
@(value1) : @"valueFor1",
@(value2) : @"valueFor2",
@(value3) : @"valueFor3",
@(value4) : @"valueFor4"
@(value5) : @"valueFor5"
};
}
return mapping[@value] ?: @"valueForDefault";
Pros
- Simpler to read, even more so if there are more values in original switch.
- Faster, mapping is created only once, then we just do quick lookup of value.
- Less prone to errors, since there is no need to do breaks/returns.
- Very easy to extract this mapping to some kind of static data like JSON, PLIST file due to nature of the code mapping.
Dynamic Mapping with blocks
How about more complicated switches, that actually do something dynamic? We can use blocks to simplify the code.
Recently I've had to refactor some code that used stringFormatting for different types:
if ([title isEqualToString:@"Scratches"])
{
title = [NSString stringWithFormat:(self.vehicle.numberOfScratches == 1 ? @"%d Scratch" : @"%d Scratches"), self.vehicle.numberOfScratches];
}
else if ([title isEqualToString:@"Dents"])
{
title = [NSString stringWithFormat:(self.vehicle.numberOfDents == 1 ? @"%d Dent" : @"%d Dents"), self.vehicle.numberOfDents];
}
else if ([title isEqualToString:@"Painted Panels"])
{
title = [NSString stringWithFormat:(self.vehicle.numberOfPaintedPanels == 1 ? @"%d Painted Panel" : @"%d Painted Panels"), self.vehicle.numberOfPaintedPanels];
}
else if ([title isEqualToString:@"Chips"])
{
title = [NSString stringWithFormat:(self.vehicle.numberOfChips == 1 ? @"%d Chip" : @"%d Chips"), self.vehicle.numberOfChips];
}
else if ([title isEqualToString:@"Tires"])
{
title = [NSString stringWithFormat:(self.vehicle.numberOfTires == 1 ? @"%d Tire" : @"%d Tires"), self.vehicle.numberOfTires];
}
else title = nil;
By using block mapping we can refactor it into something like this:
static NSDictionary *titleMapping = nil;
if (!titleMapping) {
NSString *(^const format)(NSUInteger, NSString *, NSString *) = ^(NSUInteger value, NSString *singular, NSString *plural) {
return [NSString stringWithFormat:@"%d %@", value, (value == 1 ? singular : plural)];
};
titleMapping = @{
@"Scratches" : ^(MyClass *target) {
return format([target numberOfScratches], @"Scratch", @"Scratches");
},
@"Dents" : ^(MyClass *target) {
return format([target numberOfDents], @"Dent", @"Dents");
},
@"Painted Panels" : ^(MyClass *target) {
return format([target numberOfPaintedPanels], @"Painted Panel", @"Painted Panels");
},
@"Chips" : ^(MyClass *target) {
return format([target numberOfChips], @"Chip", @"Chips");
},
@"Tires" : ^(MyClass *target) {
return format([target numberOfTires], @"Tire", @"Tires");
}
};
}
NSString *(^getTitle)(MyClass *target) = titleMapping[title];
return getTitle ? getTitle(self) : nil;
Pros
- Safer since there is no way of screwing up if-else chain
- Cached mapping since we use static variable
- We could easily add a macro to make it even smaller code foot-print and thus easier to expand
PS. I could've used string matching to implement it with even less code but I didn't think it will make it more readable.
Simpler flow by using early returns and reversing if's
As you probably figured out by now I don't like too many if's and I hate long if-else chains. Instead I prefer to keep if statements as simple as possible and use early returns.
Instead of:
if(!error) {
//! success code
} else {
//! failure code
}
I'd probably write:
if(error) {
//! failure code
return;
}
//! success code
Pros
- I don't need to read further code if I'm only interested in error case.
- In longer code I don't need to remember all the flow since I can clearly see early returns/break
Using dynamic method resolution
Sometimes we might see code similar to this:
if([type isEqualToString:@"videoWidget"]) {
[self parseVideoWidget:dictionary];
} else
if([type isEqualToString:@"imageWidget"]) {
[self parseImageWidget:dictionary];
} else
if([type isEqualToString:@"textWidget"]) {
[self parseTextWidget:dictionary];
} else
if([type isEqualToString:@"twitterWidget"]) {
[self parseTwitterWidget:dictionary];
}
First of all: I could apply all of the above refactorings to this, but this code looks like it's prime canditate for future expansion, let's see how we can make it even nicer.
SEL dynamicSelector = NSSelectorFromString([NSString stringWithFormat:@"parse%@:", type]);
if(![self respondsToSelector:dynamicSelector]) {
DDLogWarning(@"Unsupported widget type %@", type);
return;
}
[self performSelector:dynamicSelector withObject:dictionary];
Pros
- Very easy to read and understand
- Safer than if statements
- Easy to expand, I can add new widgets types in categories, eg. when doing a spin-off app I don't even need to touch the base class
Conclusion
I constantly refactor my code, this are just few tricks that are less-known but can be helpful to make your code simpler.
I usually write my code in AppCode which is a great IDE that has lots of refactoring functions that I use every-day, check it out.