This is a continuation of the blog post 12 Principles of Good Software Design.

5. An idiom should be unobtrusive and not clever.

As software developers we love to solve hard problems. The best software developers out there started with a desire to solve interesting and complicated puzzles or with the desire to make things. We enjoy the mental struggle of figuring out how to put something together, to make something appear on the screen, to wire up a bit of functionality–and when finally that button appears, the transaction is received from a remote server, the LED lights up on our embedded board–its like winning a small jackpot. Our hard work and effort allowed us to do something cool, something futuristic, something that may impact the world.

And it’s hard not to get caught up in the latest and greatest, solving problems using all the tools at our disposal. There are a lot of clever things we can do to make our code seem simpler, to make our code seem more compact, to streamline the process of writing new features. We even share those hacks with others, promulgating open source libraries which helped us solve our problem in a particularly clever way, using obscure tricks and particularly interesting ways to handle things to make our code seem “automagic”.

In some ways we’re crying out “see how clever I am?” rather than solving real problems.

I admit, I’ve done it myself.

The problem with clever solutions is that they often leverage relatively obscure language features in order to hide functionality which makes it harder for another developer to understand what is going on.

For example, here’s a particularly clever way to handle binding data fields to their associated views using KVO and some degree of introspection during the loading phase of a view controller. From our previous example, we build a binding class which allows changes in our model data to be automatically detected and used to populate the field which displays its value. This class had the following declaration:

@interface Binder : NSObject
- (void)bindKeyPath:(NSString *)path ofObject:(NSObject *)object withCallback:(void (^)(void))callback;
@end

And to use it in our ViewController, we explicitly watch for changes in our model in order to update its contents:

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Load the contents from our server
	 */

	self.balance.text = @"";
	self.change.text = @"";
	self.viewModel = [[ViewModel alloc] init];
	
	self.binder = [[Binder alloc] init];
	[self.binder bindKeyPath:@"balance" ofObject:self.viewModel withCallback:^{
		double val = self.viewModel.balance;
		self.balance.text = [NSString stringWithFormat:@"%.2f",val];
	}];
}

But as we noted in the previous example, this does not reduce the amount of code that exists in our ViewController. We still have to populate the contents of our view when we detect changes in the model.

We can update our binder, to make the binding automatic. By associating values with our views within a view controller, we could recursively iterate through all of the views within a view controller, tying the view model fields in our view controller against those views.

In this example, I will go one step further and tie the edit field of our original view controller to a two-way association: if the value inside the model changes, our text field updates. But if we type in the text field, we also update the view model contents as well; removing the responsibility from the view controller from dealing with extracting and parsing the contents of the text field.

To do this we first want to alter the signature of our binder class:

@interface Binder : NSObject
- (void)bindToViewController:(UIViewController *)vc;
@end

This allows us to rewrite our view controller as follows, removing any mention of the views within it (as the binding are automagically handled for us):

#import "ViewController.h"
#import "ViewModel.h"
#import "Binder.h"

@interface ViewController () <UITextFieldDelegate>
@property (strong, nonatomic) ViewModel *viewModel;
@property (strong, nonatomic) Binder *binder;

@end

@implementation ViewController

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Create and bind our view model
	 */

	self.viewModel = [[ViewModel alloc] init];
	self.binder = [[Binder alloc] init];
	[self.binder bindToViewController:self];
}

- (BOOL)textFieldShouldReturn:(UITextField *)textField
{
	/*
	 *	Check to make sure our balance has been properly adjusted
	 */

	NSInteger err = [self.viewModel updateBalance];
	if (err == UPDATEERROR_ILLEGAL) {
		/*
		 *	This would draw to zero. Alert the user rather than make the
		 *	round trip to the server
		 */

		UIAlertController *c = [UIAlertController alertControllerWithTitle:@"Illegal value" message:@"You may not reset the balance below zero" preferredStyle:UIAlertControllerStyleAlert];

		UIAlertAction *action = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:nil];
		[c addAction:action];

		[self presentViewController:c animated:YES completion:nil];
	}

	return NO;
}

@end

Notice that we have completely divorced the content of our view controller from behaviors handled by our view controller; binding is now being handled within our Binder class.

In order to indicate which fields for our views are associated with our model, we need to create a category which allows us to set the user defined runtime attributes associated with our views. We do that with a category which makes use of the Objective C runtime routine objc_setAssociatedObject:

#import <UIKit/UIKit.h>

@interface UIView (MVVMParams)

- (NSString *)modelField;
- (void)setModelField:(NSString *)modelField;

- (NSString *)modelType;
- (void)setModelType:(NSString *)modelType;

- (NSObject *)model;
- (void)setModel:(NSObject *)model;

@end

The implementation of this category is relatively trivial:

#import "UIView+MVVMParams.h"
#import <objc/runtime.h>

/*
 *	This category uses associations to store our parameters to tie in this
 *	view to the model field and how to handle model changes
 */

static const void *ModelFieldKey = &ModelFieldKey;
static const void *ModelTypeKey = &ModelTypeKey;
static const void *ModelKey = &ModelKey;

@implementation UIView (MVVMParams)

- (NSString *)modelField
{
	return objc_getAssociatedObject(self,ModelFieldKey);
}

- (void)setModelField:(NSString *)modelField
{
	objc_setAssociatedObject(self, ModelFieldKey, modelField, OBJC_ASSOCIATION_COPY);
}

- (NSString *)modelType
{
	return objc_getAssociatedObject(self,ModelTypeKey);
}

- (void)setModelType:(NSString *)modelType
{
	objc_setAssociatedObject(self, ModelTypeKey, modelType, OBJC_ASSOCIATION_COPY);
}

- (NSObject *)model
{
	return objc_getAssociatedObject(self,ModelKey);
}

- (void)setModel:(NSObject *)model
{
	objc_setAssociatedObject(self, ModelKey, model, OBJC_ASSOCIATION_RETAIN);
}


@end

Note that each of the parameters we set (two from within the NIB editor and one which is used by our Binder) simply store the value with an association object.

What this allows us to do is to indicate the model field and model type in our NIB file. So, for example, with our UILabel, within Xcode, we can do the following:

The category will be invoked as the view is loaded, tying the key path to the stored object. This now allows us to tie our label to the model field ‘balance’, and we specify the type as ‘fpint’. (Really, ‘fpint’ could have been anything; it’s used by our Binder code to determine how to handle the field as we will see below.)

We can now expand our ViewModel as such:

#import <Foundation/Foundation.h>

// Possible updateBalance return codes
#define UPDATEERROR_OK		0
#define UPDATEERROR_ILLEGAL	1

@interface ViewModel : NSObject
@property (nonatomic, assign) double balance;
@property (nonatomic, assign) double change;

- (void)reload;			// Reload balance
- (NSInteger)updateBalance;
@end

Notice we now have a new field, change, which will be tied via our binder to the change text field in our example. And notice that because of this, we no longer pass in a change amount to the updateBalance method; change dynamically updates as we type; updateBalance is simply triggered when it’s time to update our balance.

Our binder class has become far more complex, of course, but it is written only once. The class needs to handle binding our view model to the views inside our view controller:

// Scans all views in the view controller, constructing appropriate binding
// actions according to the parameters associated with the view.
- (void)bindToViewController:(UIViewController *)vc
{
	// First, make sure we can obtain the model
	if (![vc respondsToSelector:@selector(viewModel)]) {
		NSLog(@"Error: view controller must store the view model as viewModel");
		return;
	}

	// Now recurse through all views to tie to VM
	NSObject *vm = [(id<VCModel>)vc viewModel];
	[self bindViewToModel:vc.view model:vm];
}

Note that we expect our view model to be stored in a field in the view controller called ‘viewModel’, and we determine if we can obtain the view model by examining if our view controller class implements the ‘viewModel’ selector. If it does, we obtain the view model (and we quiet compiler errors by implementing a couple of protocols), and then we bind by recursing through all the views.

(The syntax sugar we use to shut the compiler up is:

@protocol VCModel <NSObject>
- (NSObject *)viewModel;
@end

@protocol VText <NSObject>
- (NSString *)text;
- (void)setText:(NSString *)text;
@end

The first allows us to obtain the viewModel field, the second allows us access to the text and setText fields of UILabel and UITextField.)

Our binder then recurses through all the views in the view controller, determining if any of them have an associated modelField and modelType. If they do, we then create the appropriate blocks to handle copying data in and out of the views to their respective model fields:

- (void)bindViewToModel:(UIView *)view model:(NSObject *)model
{
	// If we set the model field, tie to our model
	if (nil != (view.modelField)) {
		/*
		 *	Determine how we tie based on the model type and the type of
		 *	the class. For now we only handle UILabels and UITextFields.
		 */

		NSString *modelField = view.modelField;
		NSString *modelType = view.modelType;

		if ([view isKindOfClass:[UILabel class]]) {
			// The association is only one-way.
			[self bindKeyPath:modelField ofObject:model withCallback:^{
				[self transferToView:(UIView<VText> *)view model:model field:modelField type:modelType];
			}];
		} else if ([view isKindOfClass:[UITextField class]]) {
			// Association is two way. First tie from model to view
			[self bindKeyPath:modelField ofObject:model withCallback:^{
				[self transferToView:(UIView<VText> *)view model:model field:modelField type:modelType];
			}];

			// Now tie from view to model. Because we can't use KVO to observe
			// changes in the text, we hook up a notification
			view.model = model;
			[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(updateModelFromKey:) name:UITextFieldTextDidChangeNotification object:view];
		}
	}

	// recurse down the rest of the views
	NSArray *children = view.subviews;
	for (UIView *v in children) {
		[self bindViewToModel:v model:model];
	}
}

Our method to transfer data from the changed model into the view is simple:

// Actually pulls the data out of the model and transforms to text by type
- (void)transferToView:(UIView<VText> *)view model:(NSObject *)model field:(NSString *)field type:(NSString *)type
{
	if ([view isFirstResponder]) return;	// if we have the focus, don't update
	if ([type isEqualToString:@"fpint"]) {
		// Extract the field by calling the appropriate object send
		NSNumber *v = [model valueForKey:field];
		view.text = [NSString stringWithFormat:@"%.2f",v.doubleValue];
	}
	// TODO: Other types here.
}

Note in our example we only handle the ‘fpint’ model type. Other types would simply extend this method.

And transfering data from our view as it is updated is equally straight forward, and equally would need to change if we added other field types:

- (void)transferFromView:(UIView<VText> *)view model:(NSObject *)model field:(NSString *)field type:(NSString *)type
{
	if ([type isEqualToString:@"fpint"]) {
		// Convert floating point value
		NSString *str = view.text;
		NSNumber *val = [NSNumber numberWithDouble:str.doubleValue];
		[model setValue:val forKey:field];
	}
}

Because we are unable to get events when the text field of a UITextField, we instead look for field change notifications. We handle those notifications here:

- (void)updateModelFromKey:(NSNotification *)n
{
	UITextField *textField = n.object;
	NSString *modelField = textField.modelField;
	NSString *modelType = textField.modelType;
	NSObject *model = textField.model;

	if (model != nil) {
		[self transferFromView:(UIView *)textField model:model field:modelField type:modelType];
	}
}

This now allows us to monitor text field changes and update our model as it does.

Our complete binder class now looks like the following:

//
//  Binder.m
//  TestPrinciples
//
//  Created by William Woody on 7/26/16.
//  Copyright © 2016 Glenview Software. All rights reserved.
//

#import "Binder.h"
#import "BinderKey.h"
#import "UIView+MVVMParams.h"
#import <objc/runtime.h>

// Syntax sugar to allow us to obtain the view model from a view controller
@protocol VCModel <NSObject>
- (NSObject *)viewModel;
@end

@protocol VText <NSObject>
- (NSString *)text;
- (void)setText:(NSString *)text;
@end

// Simple binder utility aids in KVO observation

@interface Binder ()
@property (strong, nonatomic) NSMutableDictionary *map;
@end

@implementation Binder

- (id)init
{
	if (nil != (self = [super init])) {
		self.map = [[NSMutableDictionary alloc] init];
	}
	return self;
}

- (void)dealloc
{
	NSArray *a = [self.map allKeys];
	for (BinderKey *b in a) {
		[b.object removeObserver:self forKeyPath:b.keyPath];
	}

	[[NSNotificationCenter defaultCenter] removeObserver:self];
}

- (void)bindKeyPath:(NSString *)path ofObject:(NSObject *)object withCallback:(void (^)(void))callback;
{
	BinderKey *k = [[BinderKey alloc] initWithObject:object keyPath:path];
	void (^copyCallback)(void) = [callback copy];
	self.map[k] = copyCallback;

	[object addObserver:self forKeyPath:path options:NSKeyValueObservingOptionNew context:nil];
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSString *,id> *)change context:(void *)context
{
	BinderKey *k = [[BinderKey alloc] initWithObject:object keyPath:keyPath];
	void (^callback)(void) = self.map[k];
	if (callback) callback();
}

// Actually pulls the data out of the model and transforms to text by type
- (void)transferToView:(UIView<VText> *)view model:(NSObject *)model field:(NSString *)field type:(NSString *)type
{
	if ([view isFirstResponder]) return;	// if we have the focus, don't update
	if ([type isEqualToString:@"fpint"]) {
		// Extract the field by calling the appropriate object send
		NSNumber *v = [model valueForKey:field];
		view.text = [NSString stringWithFormat:@"%.2f",v.doubleValue];
	}
	// TODO: Other types here.
}

- (void)transferFromView:(UIView<VText> *)view model:(NSObject *)model field:(NSString *)field type:(NSString *)type
{
	if ([type isEqualToString:@"fpint"]) {
		// Convert floating point value
		NSString *str = view.text;
		NSNumber *val = [NSNumber numberWithDouble:str.doubleValue];
		[model setValue:val forKey:field];
	}
}

- (void)updateModelFromKey:(NSNotification *)n
{
	UITextField *textField = n.object;
	NSString *modelField = textField.modelField;
	NSString *modelType = textField.modelType;
	NSObject *model = textField.model;

	if (model != nil) {
		[self transferFromView:(UIView<VText> *)textField model:model field:modelField type:modelType];
	}
}

// Recurses through views to make appropriate ties to view model
- (void)bindViewToModel:(UIView *)view model:(NSObject *)model
{
	// If we set the model field, tie to our model
	if (nil != (view.modelField)) {
		/*
		 *	Determine how we tie based on the model type and the type of
		 *	the class. For now we only handle UILabels and UITextFields.
		 */

		NSString *modelField = view.modelField;
		NSString *modelType = view.modelType;

		if ([view isKindOfClass:[UILabel class]]) {
			// The association is only one-way.
			[self bindKeyPath:modelField ofObject:model withCallback:^{
				[self transferToView:(UIView<VText> *)view model:model field:modelField type:modelType];
			}];
		} else if ([view isKindOfClass:[UITextField class]]) {
			// Association is two way. First tie from model to view
			[self bindKeyPath:modelField ofObject:model withCallback:^{
				[self transferToView:(UIView<VText> *)view model:model field:modelField type:modelType];
			}];

			// Now tie from view to model. Because we can't use KVO to observe
			// changes in the text, we hook up a notification
			view.model = model;
			[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(updateModelFromKey:) name:UITextFieldTextDidChangeNotification object:view];
		}
	}

	// recurse down the rest of the views
	NSArray<UIView *> *children = view.subviews;
	for (UIView *v in children) {
		[self bindViewToModel:v model:model];
	}
}

// Scans all views in the view controller, constructing appropriate binding
// actions according to the parameters associated with the view.
- (void)bindToViewController:(UIViewController *)vc
{
	// First, make sure we can obtain the model
	if (![vc respondsToSelector:@selector(viewModel)]) {
		NSLog(@"Error: view controller must store the view model as viewModel");
		return;
	}

	// Now recurse through all views to tie to VM
	NSObject *vm = [(id<VCModel>)vc viewModel];
	[self bindViewToModel:vc.view model:vm];
}

@end

Our solution is extraordinarily clever, in that we’ve completely removed the responsibility for binding the views within our view controller to the view model. And notice the apparent elegance of our view controller:

#import "ViewController.h"
#import "ViewModel.h"
#import "Binder.h"

@interface ViewController () <UITextFieldDelegate>
@property (strong, nonatomic) ViewModel *viewModel;
@property (strong, nonatomic) Binder *binder;

@end

@implementation ViewController

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Create and bind our view model
	 */

	self.viewModel = [[ViewModel alloc] init];
	self.binder = [[Binder alloc] init];
	[self.binder bindToViewController:self];
}

- (BOOL)textFieldShouldReturn:(UITextField *)textField
{
	/*
	 *	Check to make sure our balance has been properly adjusted
	 */

	NSInteger err = [self.viewModel updateBalance];
	if (err == UPDATEERROR_ILLEGAL) {
		/*
		 *	This would draw to zero. Alert the user rather than make the
		 *	round trip to the server
		 */

		UIAlertController *c = [UIAlertController alertControllerWithTitle:@"Illegal value" message:@"You may not reset the balance below zero" preferredStyle:UIAlertControllerStyleAlert];

		UIAlertAction *action = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:nil];
		[c addAction:action];

		[self presentViewController:c animated:YES completion:nil];
	}

	return NO;
}

@end

So what is wrong with being clever?

First, imagine you are a new developer to the project, and you are not familiar with the source kit, or with the way Objective C allows you to do introspection.

And you see an application that presents a screen like the one below:

Then you look at the view controller above.

If you are familiar with the MVVM model, you see the ViewModel object being used, and you look at that class:

@interface ViewModel : NSObject
@property (nonatomic, assign) double balance;
@property (nonatomic, assign) double change;

- (void)reload;			// Reload balance
- (NSInteger)updateBalance;
@end

And what is the first thing you are going to ask?

Where are the properties in the View Controller which access the views?

There is no code that seems to populate the balance. There is no code that appears to update the balance. There is no code that appears to obtain the text field contents when the return button is pressed.

And the ViewModel class is only slightly helpful: sure, you see the logic to get the balance from the server, and you see the code to update the balance and perform run-time error checking.

But the rest is hidden behind this black box called Binder.

And worse, who ever thinks to look at the User Defined Runtime Attributes area for a particular view within the NIB editor? Even if you look there, what does “modelField” and “modelType” even mean?

And notice the problems with the code.

It’s fragile.

If the name of the fields within the ViewModel class change, you must hunt down where the fields are used within the view controller layout within the NIB to determine the associations.

It’s opaque

How would you know–without extensive documentation–that a ViewController must implement the ‘viewModel’ method in order to allow the binder to obtain the contents of the view model for binding?

There is no way you could know that the following is a fatal problem:

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Create and bind our view model
	 */

	self.binder = [[Binder alloc] init];
	[self.binder bindToViewController:self];
	self.viewModel = [[ViewModel alloc] init];
}

That’s because the binder class requires the viewModel field to be initialized prior to the call to -bindToViewController:, and there is no way to know this simply by examining the initialization code.

And if you do make this mistake, what is the error you get?

*** Terminating app due to uncaught exception ‘NSUnknownKeyException’, reason: ‘[<ViewController 0x7b11c540> setValue:forUndefinedKey:]: this class is not key value coding-compliant for the key change.’

Very descriptive. </sarcasm>

In being clever we introduce a variety of opportunities for bugs.

What prevents a simple typo buried deeply in the NIB from breaking the whole scheme? What if on the UILabel we accidentally misspelled “balance” as “balances”? Or use “fbInt” instead of “fbint”? We can only catch these obscure issues via a through exercise of the entire user interface–and many teams simply do not have time to do this.

It’s not “future-proof.”

What happens to the helper category if Apple introduces a new version of the operating system which adds the ‘model’ property to certain subviews, for example?

The problem with this code is that it is not obvious. It is unclear. It’s clever to the point of intruding on how we think about view controllers. And it makes use of some fragile declarations which could easily break in a future version of the iPhone operating system.

The meaning of the code has been hidden from us behind a clever but fragile construct, making it nearly impossible for another developer to maintain.

And for what? Removing a dozen lines of code from a View Controller out of some architectural deal, solving a problem that didn’t really exist in the first place?

Do you know what terrifies me about this post? Someone will think my Binder class is so cool they will flesh it out (the complete code can be found in this post and an earlier one), and post it to Github, crediting me for such a cool and clever tool.

(Shakes head)

This is a continuation of the blog post 12 Principles of Good Software Design.

4. An idiom should make your code easier to read

When you write code, you actually have two audiences, two “readers” who will be reading your software.

The first audience is the environment of compiler tools, preprocessing tools, analysis tools and other components of your development system which convert your code into a working application. That first audience is impartial, and only cares about the results of your authorship: does the compiled program run, does it meet the requirements, can testers break your code, does QA validate your code’s correctness.

For many of us, the reason why we got into software development was to write code for that impartial first audience. That audience knows no political values, is entirely emotionless. It allows us to solve difficult puzzles and tells us if our answers are right or wrong without any judgement beyond correctness. And for many of us, the fact that software development is one of the last professions which gives you a nearly guaranteed ticket to the upper-middle class is just a side benefit; it allows us to live in some degree of comfort as we solve increasingly interesting puzzles.

But there is a second audience for our code, one which many of us neglect.

Our fellow programmers.

And in many ways, writing code is very similar to writing an essay. You have related blocks of thought which are designed to hammer a single conclusion. You have varying ideas that are brought together into a single idea. You have unrelated topics that you may bounce around that eventually come together to tell a story.

All of this contributing to clarity and to communicating an idea.

Thus far, in the previous three essays covering making code simpler, making code concise and making code clear, we’ve incidentally also contributed to making code more readable. For this essay I’d like to concentrate on smaller idioms; on ways we can make code more readable that doesn’t necessarily employ using design patterns or on the choice of idioms as we refactor code.

One way we can make code more clear is in our choice of variable names.

It’s such a small thing, a variable name. But it can have a serious impact on our understanding of the code.

Now, compilers don’t care about variable names; only that you use those variables consistently. That’s how code obfuscation works with languages such as Javascript; the code remains the same, but the variables are renamed to obscure meaning to a human reader. As far as a compiler is concerned the declarations below:

#define __A long
struct __a {
	__A __b;
	__A __c;
	__A __d;
	__A __e;
};

is identical to the following declaration:

#define Integer long
struct Rect {
	Integer left;
	Integer top;
	Integer width;
	Integer height;
};

And if we were to write code that manipulates rectangles, either declaration would be fine, as far as the compiler is concerned.

Other readers of our code, however, may become puzzled when they see us manipulate rectangles using the __a declaration.

There are many naming conventions out there for variables, and many theories about why you should name variables using those different conventions. I don’t wish to come down on any side in those arguments, except to note three things.

First, a number of older conventions arose because of compiler limitations. For example, earlier C compiler linkers had limits on the maximum number of characters in a variable name that was stored. Some early C compilers only kept track of the first 6 characters or the first 32 characters of a variable name; thus, they could not tell the difference between aPointInTime and aPointOnTheLine, as compilers only storing the first 6 characters for a variable name would store aPoint for both variables.

(This is why in some early programs you see variable names like a11PointInTime or a12PointOnTheLine.)

I don’t believe any of these early tool chains exist anymore, so naming conventions which attempt to make the first N characters of a variable name (for some number N) no longer are relevant.

Hungarian Notation also arose from compiler limitations; in this case, from limitations in the type system of earlier compilers which could not distinguish between different variable types. However, again, earlier compilers have been replaced by compiles with a stronger type system, and variable naming conventions such as these place an additional burden on the author of the code (and by extension, on the reader) that is no longer quite as important.

Second, there are limits on the ability for a reader to read long variable names. There is a reason why Einstein’s famous equation is not written:

but rather, in the more compact form:

This, despite the fact that the former representation is arguably more informative.

Book layout editors and newspaper authors both know that the human eye can only see so much, can only scan so much distance across a line of text and successfully return to the next line, can only see so much text in a single word. Words like “antidisestablishmentarianism” exist mostly as curiosities in an essay such as mine where the average word length is less than 6 characters.

It’s not to suggest that we should go to the other extreme, that we should go to a FORTRAN-style naming convention or a naming convention from early forms of BASIC where all variable names were kept to two characters or less. But it is to suggest that a concise variable name may be easier for someone to follow–assuming the name is descriptive enough to describe what the variable holds.

In some places, I personally use short variable names. I grew up in an era where integer loop variables were i, j or k; sometimes I abbreviate temporary variables holding the length of a thing len, or a variable holding a single character as ch. But these are immediate temporary variables used only within local scope, and by using such short variable names I hope to distinguish between them and class-scope variables such as submitButton or cachedLength.

But this is my style, just as my essay writing style incorporates run-on sentences, sometimes punctuated with short sentences to drive a point home. There is a place for personal style.

So long as we convey the idea we wish to convey in a clear and concise manner, we’ve accomplished our purpose.

Third, consistent naming conventions help with understanding. You can see this all throughout Apple’s development libraries. From the earliest days of Inside Macintosh, Apple used standard naming conventions for their toolbox functions: open balances with close, new balances with dispose, create balances with delete, begin balances with end.

In more recent versions of MacOS X and iOS prior to ARC, method names starting with alloc, new, or containing copy would pass ownership to the caller and would need to be released either using the release or autorelease methods. This naming convention is enforced by ARC and is also enforced by Apple’s Xcode analysis tools.

Naming conventions are important, because they enforce consistency and help developers maintaining your code remember what are the correct methods to call when extending your code. It’s not necessary to use Apple’s naming conventions; what is important is to stick to a naming convention in a consistent fashion throughout your code.

If you use a naming convention such as the one used in Java, where routines that start with ‘get’ get the contents of a variable name (without side effects), and ‘set’ sets the value of a variable name (without meaningful side effects), then stick to that naming convention. Don’t suddenly create a new class like the following:

public class MyThing
{
	private int __value;
	
	public int value()
	{
		return __value;
	}
	
	public void updateValue(int v)
	{
		__value = v;
	}
}

Also, it’s important to be aware of any naming conventions in the environment you are using. For example, while Java uses ‘get’ as the getter for a variable held by a class, Objective C simply uses the name of the variable:

@implmentation MyThing
- (void)setValue:(int)v
{
	self.internalValue = v;
}
- (int)value
{
	return self.internalValue;
}
@end

If you are creating a class which manages resources, you may wish to use a consistent naming convention throughout. So, for example, like in the Macintosh System Toolbox, creating a window is done using ‘newWindow’; releasing is done with ‘disposeWindow’. Creating a new resource is done using ‘newResource’; releasing with ‘disposeResource’. And this consistency allows a new developer who sees a chunk of code creating a new scratch database with the method ‘newScratchDatabase’ can know with some degree of certainty the release call will be ‘disposeScratchDatabase.’

Beyond just naming conventions there are other ways you can aid with readability of your code. Like this essay, code has “sections”, “paragraphs” (which group functionality), and “sentences” or statements which describe an idea. Your choice of how you organize your sentences within a paragraph can aid in readability, even if you do not choose to refactor your code into separate functions or by using a design pattern. White space can be a powerful tool in comprehension.

For example, take the following code from a previous essay:

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Set up the appearance of our interface
	 */

	self.firstButton.layer.borderColor = [UIColor lightGrayColor].CGColor;
	self.secondButton.layer.borderColor = [UIColor lightGrayColor].CGColor;
	self.firstButton.layer.borderWidth = 1;
	self.secondButton.layer.borderWidth = 1;
	self.thirdButton.layer.borderWidth = 2;
	self.thirdButton.layer.borderColor = [UIColor darkGrayColor].CGColor;
	self.firstButton.layer.cornerRadius = 6;
	self.secondButton.layer.cornerRadius = 6;
	self.thirdButton.layer.cornerRadius = 6;
}

The clump of code at the bottom initializes three buttons, one representing a “default” button. Our previous solution involved refactoring the code by removing common functionality into a separate method call.

But we can make our code above more readable simply by reordering our statements and adding some white space:

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Set up the appearance of our interface
	 */

	self.firstButton.layer.borderColor = [UIColor lightGrayColor].CGColor;
	self.firstButton.layer.borderWidth = 1;
	self.firstButton.layer.cornerRadius = 6;

	self.secondButton.layer.borderColor = [UIColor lightGrayColor].CGColor;
	self.secondButton.layer.borderWidth = 1;
	self.secondButton.layer.cornerRadius = 6;

	self.thirdButton.layer.borderColor = [UIColor darkGrayColor].CGColor;
	self.thirdButton.layer.borderWidth = 2;
	self.thirdButton.layer.cornerRadius = 6;
}

This is far more readable than the previous example, and all we did was clump calls which affected buttons into their own block of code, reordering the statements so that they match, and adding white space to help visually separate clumps of code.

In fact, this should be the first step in refactoring our code into a separate method call.

Run-on statements can similarly be broken down in order to aid with understanding. With earlier compilers, run-on statements helped the compiler optimize for performance, but today’s optimizing compilers do not require collapsing statements into more compact forms in order to optimize the code. In fact, certain statement representations may hinder code optimization, and rewriting run-on statements can help the compiler by reducing statement complexity.

Take, for example, the following statement which draws text within a rectangle:

- (void)draw
{
	[UIColor.blackColor setStroke];
	[[UIBezierPath bezierPathWithRect: CGRectMake(39, 31, 88, 30)] stroke];
	[@"This is a test." drawInRect: CGRectMake(39, 31, 88, 30) withAttributes: @{ NSFontAttributeName: [UIFont fontWithName: @"HelveticaNeue" size: 12], NSForegroundColorAttributeName: UIColor.blackColor }];
}

While compact, this code is hard to follow, and has the problem of repeating code which initializes the rectangle we are drawing into. We can aid in the readability of this code by expanding the run-on statements:

- (void)draw
{
	CGRect rect = CGRectMake(39, 31, 88, 30);

	[UIColor.blackColor setStroke];
	UIBezierPath *path = [UIBezierPath bezierPathWithRect: rect];
	[path stroke];

	UIFont *font = [UIFont fontWithName: @"HelveticaNeue" size: 12];
	UIColor *color = UIColor.blackColor;
	NSDictionary *attrs = @{ NSFontAttributeName: font,
							 NSForegroundColorAttributeName: color };
	[@"This is a test." drawInRect: rect withAttributes: attrs];
}

While taking more lines, we can now see where we are creating our rectangle path, where we are creating our font, our color for our text, and we can now understand all the side effects of our code. We even collapsed the two rectangle creation statements into one, thereby reducing the potential for error.

From the latter representation we now have a better understanding–and we can contemplate further refactoring steps, such as passing in the text or rectangle as parameters, or manipulating the location of the text to be centered in the rectangle. We could also obtain the text attributes from a separate function call (so we consistently use the same text attributes in other places in our code).

Sometimes smaller isn’t better.

In the end it doesn’t matter the naming conventions you use. What is important is that the convention is used with consistency; consistency aids with readability.

And as long as you remember that you are writing for an audience of fellow programmers, and write your code in such a way as to make your code easier to read, then you can help your audience in their understanding as they take over your source kit to maintain it in the future.

This is a continuation of the blog post 12 Principles of Good Software Design.

3. An idiom should make the intent of your code clear

I’ve seen in a number of places a number of rules for writing good code. Many of the rules revolve around having a class method do only one thing, or making sure functions have no side effects or that classes can be described with an “elevator pitch”; described in 10 seconds or less to someone who hasn’t a clue.

All of these ideas work because they help make clear the intent of your code.

Clarifying the intent of your code is important for a variety of reasons. By clarifying the intent of your code you make the intended functionality clear to yourself and to others who may work on your software. Clarifying the intent helps you organize your code in such a way so that it can be more easily maintained. And in striving for clear code you keep unnecessary complexity from creeping in.

There are a number of other ways to clarify the intent of your code.

Here’s a simple example. We have three buttons on iOS. In our -viewDidLoad: method we set up the user interface. One button stands out as the default button, the other two have lighter borders.

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Set up the appearance of our interface
	 */

	self.firstButton.layer.borderColor = [UIColor lightGrayColor].CGColor;
	self.secondButton.layer.borderColor = [UIColor lightGrayColor].CGColor;
	self.firstButton.layer.borderWidth = 1;
	self.secondButton.layer.borderWidth = 1;
	self.thirdButton.layer.borderWidth = 2;
	self.thirdButton.layer.borderColor = [UIColor darkGrayColor].CGColor;
	self.firstButton.layer.cornerRadius = 6;
	self.secondButton.layer.cornerRadius = 6;
	self.thirdButton.layer.cornerRadius = 6;
}

Sure, the purpose of the -viewDidLoad: method is to initialize the user interface. But notice we’re doing more or less the same thing for each of the buttons. If we were to take the initialization and roll it into a separate method we can make the intent of our setup code a lot more clear.

First, refactor our common startup in a separate method:

+ (void)setupButton:(UIButton *)button asDefault:(BOOL)defFlag
{
	button.layer.borderColor = defFlag ? [UIColor darkGrayColor].CGColor : [UIColor lightGrayColor].CGColor;
	button.layer.borderWidth = defFlag ? 2 : 1;
	button.layer.cornerRadius = 6;
}

Now our startup code in -viewDidLoad: becomes far more apparent:

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Set up the apperance of our interface
	 */

	[ViewController setupButton:self.firstButton  asDefault:NO];
	[ViewController setupButton:self.secondButton asDefault:NO];
	[ViewController setupButton:self.thirdButton  asDefault:YES];
}

We can readily see the intent of our 9 lines of initialization code.

And the advantage of expressing our intent in this way is that, in the future, if our requirements for the appearance of our buttons change, we only need to change the +setupButton:asDefault: method, as we’ve separated the semantics of how the button is used (asDefault:YES verses NO) from the actual appearance.

We can find another example from older versions of iOS. The UIAlertView class has a method for a delegate to receive a callback if the user dismisses the alert using a particular button. This allows you to prompt a user, and take action if they select “OK” from the alert.

For example, suppose we have a method which prompts the user to save data:

- (void)shouldSave
{
	UIAlertView *v = [[UIAlertView alloc] initWithTitle:@"Save?" 
			message:@"Should this file be saved?" 
			delegate:self 
			cancelButtonTitle:@"Cancel" 
			otherButtonTitles:@"Save", nil];
	[v show];
}

The problem is that we only show half of the intent of our code. Meaning the real intent of our code is to prompt the user, then to take action if the user responds in a particular way. Yet we only see half the intent; the other half is a few lines or a few hundred lines away:

- (void)alertView:(UIAlertView *)alertView clickedButtonAtIndex:(NSInteger)buttonIndex
{
	if (buttonIndex == alertView.firstOtherButtonIndex) {
		( Do save operation )
	}
}

It gets worse if we have multiple alert views within a single controller; if we use our view controller as the single delegate for multiple alerts, we have to set some parameter in the alert, then create a switch statement to figure out which alert we were displaying.

Awkward.

Yet a simple helper class to allow us to get a call back in an Objective C block when an alert button is pressed can help us keep the code together–making the intent of our code more obvious.

Our AlertView class extends the UIAlertView with a new method:

@interface AlertView : UIAlertView<UIAlertViewDelegate>
- (void)showWithCallback:(void (^)(NSInteger btn))callback;
@end

The implementation is straight forward: the alert uses itself as the delegate, and passes the click button to the block provided:

#import "AlertView.h"

@interface AlertView ()
@property (copy) void (^callback)(NSInteger btn);
@end

@implementation AlertView

- (void)showWithCallback:(void (^)(NSInteger btn))cb
{
	self.callback = cb;
	self.delegate = self;
	[self show];
}

- (void)alertView:(UIAlertView *)alertView clickedButtonAtIndex:(NSInteger)buttonIndex
{
	self.callback(buttonIndex);
}

@end

Now we can make the intent of our alert code more clear:

- (void)shouldSave
{
	AlertView *v = [[AlertView alloc] 
			initWithTitle:@"Save?" 
			message:@"Should this file be saved?" 
			delegate:nil
			cancelButtonTitle:@"Cancel" 
			otherButtonTitles:@"Save", nil];
	[v showWithCallback:^(NSInteger buttonIndex) {
		if (buttonIndex == v.firstOtherButtonIndex) {
			// Do save operation
		}
	}];
}

The intent is far clearer because the action of displaying the alert, and the code which handles the results of the alert, are now all together in one place.

Sometimes the intent of your code can be hard to convey because it implements a complex algorithm or does something unusual. But most of the applications we are asked to write are relatively simple: a screen is presented and the contents loaded from some source. You alter the contents of the data represented on the screen. The contents are then saved, and any errors along the way are displayed to the user.

Simple.

And by keeping the intent clear of each component we create, from setting up our user interface to saving the contents of our screen, you make it easier to maintain your code.

This is a continuation of the blog post 12 Principles of Good Software Design.

2. An idiom should make your code concise

Suppose you have the task of creating an API for your application which can communicate with a back end server. For each API call you need to signal a wait spinner, assemble a request to the back end server, send the request, wait for the response, and parse the response in JSON, signaling a library if there is a server error so it can present the user an error message.

Suppose that our server takes its arguments as JSON as part of a POST payload. Thus, our requests look like this:

POST http://api.server.co/api/1/endpointname
{ "arg1": value_1,
  "arg2": value_2 ... }

Our endpoint “endpointname” requires two arguments, “arg1” and “arg2”, which pass in integer values.

The result, for our example, looks like this:

{ "result": result }

As a first pass, we could build a class Server which creates the NSURLSession object on iOS which allows us to make a network request and get a response. The declaration looks like this:

@interface Server : NSObject

+ (Server *)shared;

- (NSURLSession *)session;

@end

The class itself simply creates a session and returns it:

#import "Server.h"

@interface Server ()
@property (strong, nonatomic) NSURLSession *session;
@end

@implementation Server

- (id)init
{
	if (nil != (self = [super init])) {
		NSURLSessionConfiguration *config = [NSURLSessionConfiguration ephemeralSessionConfiguration];
		self.session = [NSURLSession sessionWithConfiguration:config];
	}
	return self;
}

+ (Server *)shared
{
	static Server *singleton;
	static dispatch_once_t onceToken;
	dispatch_once(&onceToken, ^{
		singleton = [[Server alloc] init];
	});
	return singleton;
}

@end

In order to make a request, we would then need to handle each of the steps above: start the wait spinner, assemble the request, send the request, and parse the response. For the example below we’ll ignore error codes and simply concentrate on the code which handles parsing the response.

Notice all the work we have to do in order to make our request:

- (void)doOperationWithCallback:(void (^)(NSInteger res))callback;
{
	void (^copyCallback)(NSInteger val) = [callback copy];

	int value1 = (do something here);
	int value2 = (do something here);

	// Start the wait spinner
	[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];

	// Assemble the request
	NSString *endpoint = @"endpointname";
	NSString *reqString = [NSString stringWithFormat:@"%@%@",SERVERPREFIX,endpoint];
	NSURL *url = [NSURL URLWithString:reqString];

	NSMutableURLRequest *req;
	req = [[NSMutableURLRequest alloc] initWithURL:url];
	[req setHTTPMethod:@"POST"];

	NSDictionary *d = @{ @"arg1": @( value1 ),
						 @"arg2": @( value2 ) };
	NSData *jsonReq = [NSJSONSerialization dataWithJSONObject:d options:NSJSONWritingPrettyPrinted error:nil];
	[req setHTTPBody:jsonReq];
	[req setValue:@"application/json" forHTTPHeaderField:@"Content-Type"];

	// Now send the request and wait for the response
	[[Server shared].session dataTaskWithRequest:req completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
		// Process the response. Note not handling errors for now.
		NSDictionary *respData = [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
		NSInteger retVal = [respData[@"result"] integerValue];

		// Dispatch back in main thread. (Our callback was in a separate
		// thread.)
		dispatch_async(dispatch_get_main_queue(), ^{
			[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];
			copyCallback(retVal);
		});
	}];
}

At every point in our code where we call the back end we have to write the same 38 lines of code. We have to keep track of the callback block. We have to start and stop the spinner. We have to create the request. We have to parse the response from JSON to something we can use. We have to put the response back on the main UI thread.

And notice the bugs: if we have two network requests at the same time we may inadvertently turn off the network activity spinner before network activity is complete. We aren’t handling errors.

This is most certainly not concise. There are things we can do to help reduce the line count.

The simplest is to observe that we are starting and stopping the network activity spinner every time we make a network request. So we can extend our Server class to incorporate managing the network activity spinner.

So we change our server class declaration by hiding the session object and exposing a method call instead:

@interface Server : NSObject

+ (Server *)shared;

- (void)request:(NSURLRequest *)req completionHandler:(void (^)(NSData *data, NSURLResponse *response, NSError *error))callback;

@end

Next, we create our request wrapper:

- (void)request:(NSURLRequest *)req completionHandler:(void (^)(NSData *data, NSURLResponse *response, NSError *error))callback
{
	void (^copyCallback)(NSData *data, NSURLResponse *response, NSError *error) = [callback copy];

	// Start the wait spinner
	[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];

	// Run the request
	[self.session dataTaskWithRequest:req completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {

		// Stop the wait spinner. Do this on the main thread
		dispatch_async(dispatch_get_main_queue(), ^{
			[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];
		});

		// Call our callback
		copyCallback(data,response,error);
	}];
}

And now we can rewrite our operation call by eliminating the wait spinner.

- (void)doOperationWithCallback:(void (^)(NSInteger res))callback;
{
	void (^copyCallback)(NSInteger val) = [callback copy];

	int value1 = (do something here);
	int value2 = (do something here);

	// Assemble the request
	NSString *endpoint = @"endpointname";
	NSString *reqString = [NSString stringWithFormat:@"%@%@",SERVERPREFIX,endpoint];
	NSURL *url = [NSURL URLWithString:reqString];

	NSMutableURLRequest *req;
	req = [[NSMutableURLRequest alloc] initWithURL:url];
	[req setHTTPMethod:@"POST"];

	NSDictionary *d = @{ @"arg1": @( value1 ),
						 @"arg2": @( value2 ) };
	NSData *jsonReq = [NSJSONSerialization dataWithJSONObject:d options:NSJSONWritingPrettyPrinted error:nil];
	[req setHTTPBody:jsonReq];
	[req setValue:@"application/json" forHTTPHeaderField:@"Content-Type"];

	// Now send the request and wait for the response
	[[Server shared].session dataTaskWithRequest:req completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
		// Process the response. Note not handling errors for now.
		NSDictionary *respData = [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
		NSInteger retVal = [respData[@"result"] integerValue];

		// Dispatch back in main thread. (Our callback was in a separate
		// thread.)
		dispatch_async(dispatch_get_main_queue(), ^{
			copyCallback(retVal);
		});
	}];
}

The code is shorter, but concise? Hardly.

There is an advantage to this code, by the way. Because our wait spinner code is now in one location rather than scattered everywhere where we make a request, we can easily update our wait spinner code to fix the bug of hiding the wait spinner prematurely when two network requests are made at the same time, by wrapping our code in a counter and hiding the spinner only when the count reaches zero.

Notice a couple of things about our server requests, however. First, our server example above always takes POST requests. Second, it always takes and returns JSON requests. Additionally we assume only the endpointname changes; the rest of the URL remains the same.

So why are we doing all this duplicate work?

Let’s move the functionality into our Server class.

First, we know we always take an endpoint name and an (optional) dictionary of parameters. And we know the return result will always be a JSON response, so instead of returning raw data we return our parsed data. So our server class looks like this:

@interface Server : NSObject

+ (Server *)shared;

- (void)requestWithEndpoint:(NSString *)endpoint
				 parameters:(NSDictionary *)params
		  completionHandler:(void (^)(NSDictionary *data, NSURLResponse *response, NSError *error))callback;

@end

Our server should now handle all the common work we scattered throughout our code:

#define SERVERPREFIX	@"http://api.server.co/api/1/"

- (void)requestWithEndpoint:(NSString *)endpoint parameters:(NSDictionary *)params completionHandler:(void (^)(NSDictionary *data, NSURLResponse *response, NSError *error))callback
{
	void (^copyCallback)(NSDictionary *data, NSURLResponse *response, NSError *error) = [callback copy];

	// Start the wait spinner
	[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];

	// Assemble the request
	NSString *reqString = [NSString stringWithFormat:@"%@%@",SERVERPREFIX,endpoint];
	NSURL *url = [NSURL URLWithString:reqString];

	NSMutableURLRequest *req;
	req = [[NSMutableURLRequest alloc] initWithURL:url];
	[req setHTTPMethod:@"POST"];

	if (params) {
		NSData *jsonReq = [NSJSONSerialization dataWithJSONObject:params options:NSJSONWritingPrettyPrinted error:nil];
		[req setHTTPBody:jsonReq];
		[req setValue:@"application/json" forHTTPHeaderField:@"Content-Type"];
	}

	// Now send the request and wait for the response
	[[Server shared].session dataTaskWithRequest:req completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
		// Process the response. Note not handling errors for now.
		NSDictionary *respData = nil;
		if (data) {
			respData = [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
		}

		// Dispatch back in main thread. (Our callback was in a separate
		// thread.)
		dispatch_async(dispatch_get_main_queue(), ^{
			[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];
			copyCallback(respData,response,error);
		});
	}];
}

Note that our single entrypoint is handling most of the common work that would otherwise be scattered throughout our code.

And our doOperationWithCallback method?

- (void)doOperationWithCallback:(void (^)(NSInteger res))callback;
{
	void (^copyCallback)(NSInteger val) = [callback copy];

	int value1 = (do something here);
	int value2 = (do something here);

	NSDictionary *d = @{ @"arg1": @( value1 ),
						 @"arg2": @( value2 ) };

	[[Server shared] requestWithEndpoint:@"endpointname" parameters:d completionHandler:^(NSDictionary *data, NSURLResponse *response, NSError *error) {
		NSInteger retVal = [data[@"result"] integerValue];
		copyCallback(retVal);
	}];
}

We’ve made our code concise.

What we’re doing has become extremely apparent, because all the duplicate bookkeeping work is now handled elsewhere in a common location.

We don’t have to worry about wait spinners or about constructing the request object–all we have to worry about is which endpoint are we calling, and what parameters are we passing, and what results we’re expecting.

An idiom should make your code concise.

Because by making your code concise you implicitly do a few things. First, you move repetitive code to another common location, where bugs (such as our wait spinner bug) can be easily fixed in one place. Second, you reveal the purpose of your function call by eliminating all the repetitive bookkeeping that distracts from the purpose of the code. And third, you reduce the probability of error.

For example, can you spot what is wrong with the following code?

- (void)doOperationWithCallback:(void (^)(NSInteger res))callback;
{
	void (^copyCallback)(NSInteger val) = [callback copy];

	int value1 = (do something here);
	int value2 = (do something here);

	// Start the wait spinner
	[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];

	// Assemble the request
	NSString *endpoint = @"endpointname";
	NSString *reqString = [NSString stringWithFormat:@"%@%@",SERVERPREFIX,endpoint];
	NSURL *url = [NSURL URLWithString:reqString];

	NSMutableURLRequest *req;
	req = [[NSMutableURLRequest alloc] initWithURL:url];
	[req setHTTPMethod:@"POST"];

	NSDictionary *d = @{ @"arg1": @( value1 ) };
	NSData *jsonReq = [NSJSONSerialization dataWithJSONObject:d options:NSJSONWritingPrettyPrinted error:nil];
	[req setHTTPBody:jsonReq];
	[req setValue:@"application/json" forHTTPHeaderField:@"Content-Type"];

	// Now send the request and wait for the response
	[[Server shared].session dataTaskWithRequest:req completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
		// Process the response. Note not handling errors for now.
		NSDictionary *respData = [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
		NSInteger retVal = [respData[@"result"] integerValue];

		// Dispatch back in main thread. (Our callback was in a separate
		// thread.)
		dispatch_async(dispatch_get_main_queue(), ^{
			[[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];
			copyCallback(retVal);
		});
	}];
}

Did you spot the error?

Think about the requirements we stated above, where our endpoint requires two arguments.

How about now? Can you spot the error now?

- (void)doOperationWithCallback:(void (^)(NSInteger res))callback;
{
	void (^copyCallback)(NSInteger val) = [callback copy];

	int value1 = (do something here);
	int value2 = (do something here);

	NSDictionary *d = @{ @"arg1": @( value1 ) };

	[[Server shared] requestWithEndpoint:@"endpointname" parameters:d completionHandler:^(NSDictionary *data, NSURLResponse *response, NSError *error) {
		NSInteger retVal = [data[@"result"] integerValue];
		copyCallback(retVal);
	}];
}

Yep, we forgot a parameter to our endpoint.

And it is easier to spot the error in 15 lines of code which reduce the process to its essentials, than in 38 lines of code full of extraneous bookkeeping code.

This is a continuation of the blog post 12 Principles of Good Software Design.

1. An idiom should make your code simpler

It is a shame that this first principle even has to be said, since it is so self-evidently true. But all too often we see people employ design patterns or architectural redesigns in the name of clarity or out of a desire for better code organization which results in making the code more complex. The theory is that eventually we’ll need the features so adding complexity now will help us in the future.

But notice what happens: the future never comes, the code is handed over to someone who is confused by the new design pattern, and they add code rather than fix what is there, because it’s better to leave something that mostly works alone than reorganize what you don’t understand.

Besides, there are only so many hours in the day, and it’s Friday, and we’ve got tickets to the latest Star Trek movie…

There is a flip side of this principle that can also be violated. Quoting Albert Einstein: “Make everything as simple as possible, but not simpler.”

Sometimes people attempt to make the code simpler than it can be made. And in the process they wind up with extra implementation details that they then “hide” using tricks or obscure language features which make it impossible to know what the software is doing.

Ironically we can see both of these (hiding features and added complexity) in the following example.

For the following example, written in Objective C for iOS, we have a single view controller. The view controller shows a balance that is stored on a remote server, and the interface to the server can be used to both obtain the current balance and update the balance by subtracting from it. Our balance cannot go below zero.

Our interface looks like:

@interface ServerModel : NSObject
+ (ServerModel *)shared;

- (void)obtainBalance:(void (^)(NSInteger bal))callback;
- (void)updateBalance:(NSInteger)update callback:(void (^)(NSInteger bal))callback;
@end

The callback method provided is called asynchronously; after all, the value has to be pulled from a server. To keep things simple for this example, we do not pass an error to the user. (In real life, of course, you would pass the error and handle network issues appropriately.)

Now when our view controller contains two controls: a label which is used to show the balance, and a text field which is used to enter a change in the balance. The return key is used to update our balance.

Our view controller looks like the following:

#import "ViewController.h"
#import "ServerModel.h"

@interface ViewController () <UITextFieldDelegate>
@property (weak, nonatomic) IBOutlet UILabel *balance;
@property (weak, nonatomic) IBOutlet UITextField *change;
@property (assign, nonatomic) double curBalance;
@end

@implementation ViewController

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Load the contents from our server
	 */

	self.balance.text = @"";
	self.change.text = @"";
	[[ServerModel shared] obtainBalance:^(NSInteger bal) {
		// Received balance; update contents
		double val = bal/100.0;
		self.curBalance = val;
		self.balance.text = [NSString stringWithFormat:@"%.2f",val];
	}];
}

- (BOOL)textFieldShouldReturn:(UITextField *)textField
{
	double change = self.change.text.doubleValue;

	/*
	 *	Check to make sure our balance has been properly adjusted
	 */

	if (self.curBalance - change < 0) {
		/*
		 *	This would draw to zero. Alert the user rather than make the
		 *	round trip to the server
		 */

		UIAlertController *c = [UIAlertController alertControllerWithTitle:@"Illegal value" message:@"You may not reset the balance below zero" preferredStyle:UIAlertControllerStyleAlert];

		UIAlertAction *action = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:nil];
		[c addAction:action];

		[self presentViewController:c animated:YES completion:nil];
		return NO;
	}

	self.change.text = @"";		// Clear the value we entered.

	/*
	 *	Sanity check against balance.
	 */

	[[ServerModel shared] updateBalance:(int)(change * 100 + 0.5) callback:^(NSInteger bal) {
		double val = bal/100.0;
		self.balance.text = [NSString stringWithFormat:@"%.2f",val];
	}];
	return NO;
}

@end

When the view loads, the balance is retrieved; some time later, we then display the balance on our display. When the user enters a value, we check to make sure the balance has not gone below zero; if it does, we warn the user. Otherwise, we update the balance and (when the updated balance returns), we update the displayed balance.

Of course there are some missing features. Our balance may be updated from a number of sources, and periodically we may want to ping the server to see if our balance changed. Our protocol may even notify us if the balance has changed unilaterally. We may want to display a “wait” spinner as we are loading the contents. We of course need to handle network errors.

Our example is 66 lines long, consisting of two methods, though one of the methods is rather long as it handles the business logic of validating the inputs locally.

Now let’s suppose we wish to refactor our code to use MVVM.

There are two motivations for MVVM. First, it presumes that the model in our applications are simplistic, and we need to push the business logic in our view controller (the actions of pulling and pushing data from the server, validating our inputs) to a separate chunk of code from our “view” (the view controller), which is solely responsible for managing the display.

The original motivation for MVVM comes from the C# world, where tools exist to help construct a view controller and tie fields of that view controller to a model, and where tools exist to build models as collections of data objects. In that world, when the tools are building our view controller and other tools are building our model, naturally the business logic needs to go somewhere else, right?

The additional motivations for MVVM in the iOS world is to help isolate the business logic in a View Controller to something that can be run in a unit test without involving the user interface itself.

So let’s do that.

First, we need a view model which handles the logic of obtaining and saving our data, as well as the business logic of validating our balance. Because this object ideally has no user interface logic, we need to alert the view model when there is an error.

Our final ViewModel object has the following declaration:

#import <Foundation/Foundation.h>

// Possible updateBalance return codes
#define UPDATEERROR_OK		0
#define UPDATEERROR_ILLEGAL	1

@interface ViewModel : NSObject
@property (nonatomic, assign) double balance;

- (void)reload;			// Reload balance
- (NSInteger)updateBalance:(double)update;
@end

Note we expose the reload method, though reload is implicitly called when our object is first constructed. We also provide an updateBalance method which returns an error code; these errors are only errors detected during pre-flight; server errors would need to be handled separately.

Our ViewModel code looks like:

#import "ViewModel.h"
#import "ServerModel.h"

@implementation ViewModel

- (id)init
{
	if (nil != (self = [super init])) {
		[self reload];
	}
	return self;
}

- (void)reload
{
	[[ServerModel shared] obtainBalance:^(NSInteger bal) {
		self.balance = bal / 100.0;
	}];
}

- (NSInteger)updateBalance:(double)update
{
	/* Preflight errors */
	if (self.balance - update < 0) {
		return UPDATEERROR_ILLEGAL;
	}

	/* Update our balance */
	[[ServerModel shared] updateBalance:(NSInteger)(update*100 + 0.5) callback:^(NSInteger bal) {
		self.balance = bal / 100.0;
	}];

	return UPDATEERROR_OK;
}

@end

This seems relatively straight forward. Our ViewModel object communicates with our server to maintain the balance internally.

And if we refactor our original ViewController to use our ViewModel class, notice that things appear simpler.

#import "ViewController.h"
#import "ViewModel.h"

@interface ViewController () <UITextFieldDelegate>
@property (weak, nonatomic) IBOutlet UILabel *balance;
@property (weak, nonatomic) IBOutlet UITextField *change;
@property (strong, nonatomic) ViewModel *viewModel;
@end

@implementation ViewController

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Load the contents from our server
	 */

	self.balance.text = @"";
	self.change.text = @"";
	self.viewModel = [[ViewModel alloc] init];
}

- (BOOL)textFieldShouldReturn:(UITextField *)textField
{
	double change = self.change.text.doubleValue;

	/*
	 *	Check to make sure our balance has been properly adjusted
	 */

	NSInteger err = [self.viewModel updateBalance:change];
	if (err == UPDATEERROR_ILLEGAL) {
		/*
		 *	This would draw to zero. Alert the user rather than make the
		 *	round trip to the server
		 */

		UIAlertController *c = [UIAlertController alertControllerWithTitle:@"Illegal value" message:@"You may not reset the balance below zero" preferredStyle:UIAlertControllerStyleAlert];

		UIAlertAction *action = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:nil];
		[c addAction:action];

		[self presentViewController:c animated:YES completion:nil];
	} else if (err == UPDATEERROR_OK) {
		self.change.text = @"";
	}

	return NO;
}

@end

However, we have a problem, and that is the balance is not reflected in the view controller’s balance label.

For this, we use KVO to observe changes in the ViewModel’s balance field. (In the Microsoft .Net environment, this binding is handled for you by a collection of built-in tools.)

There are plenty of KVO libraries out there (such as ReactiveCocoa) which provide a variety of classes and methods which aid in this, but let’s go ahead–as this is such as simple example–and handle it ourselves.

In order to simplify the binding operation, we create a class called “Binder”. Our class is simple: it handles all the bindings and issues a call to a block when the value our binder is bound to changes. The binder releases all of the observers on the object when it is disposed of, so we only need to create the binder in our ViewController, and bind against the values whose values we are interested in monitoring.

Our binder class has the following declaration:

#import <Foundation/Foundation.h>

@interface Binder : NSObject
- (void)bindKeyPath:(NSString *)path ofObject:(NSObject *)object withCallback:(void (^)(void))callback;
@end

And the declaration of our binder class is:

#import "Binder.h"
#import "BinderKey.h"

// Simple binder utility aids in KVO observation

@interface Binder ()
@property (strong, nonatomic) NSMutableDictionary *map;
@end

@implementation Binder

- (id)init
{
	if (nil != (self = [super init])) {
		self.map = [[NSMutableDictionary alloc] init];
	}
	return self;
}

- (void)dealloc
{
	NSArray *a = [self.map allKeys];
	for (BinderKey *b in a) {
		[b.object removeObserver:self forKeyPath:b.keyPath];
	}
}

- (void)bindKeyPath:(NSString *)path ofObject:(NSObject *)object withCallback:(void (^)(void))callback;
{
	BinderKey *k = [[BinderKey alloc] initWithObject:object keyPath:path];
	void (^copyCallback)(void) = [callback copy];
	self.map[k] = copyCallback;

	[object addObserver:self forKeyPath:path options:NSKeyValueObservingOptionNew context:nil];
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
{
	BinderKey *k = [[BinderKey alloc] initWithObject:object keyPath:keyPath];
	void (^callback)(void) = self.map[k];
	if (callback) callback();
}

@end

Note our class uses a dictionary to rapidly look up a particular callback block given an object and the keypath being observed. This uses a custom dictionary key, whose declaration is:

#import <Foundation/Foundation.h>

// A key used internally for my NSDictionary by my binder. This tracks
// a specific object (by pointer) and a specific key
@interface BinderKey : NSObject <NSCopying>
- (id)initWithObject:(NSObject *)obj keyPath:(NSString *)key;
- (NSObject *)object;
- (NSString *)keyPath;
@end

And the body is:

#import "BinderKey.h"

@interface BinderKey()
@property (weak, nonatomic) NSObject *object;
@property (copy, nonatomic) NSString *keyPath;
@end

@implementation BinderKey
- (id)initWithObject:(NSObject *)obj keyPath:(NSString *)key;
{
	if (nil != (self = [super init])) {
		self.object = obj;
		self.keyPath = key;
	}
	return self;
}

- (id)copyWithZone:(NSZone *)zone
{
	// We are a constant object so we just return ourselves.
	return self;
}

- (NSUInteger)hash
{
	NSUInteger ptr = (NSUInteger)(self.object);		// object pointer equality
	return [self.keyPath hash] ^ ptr;				// silly hash code
}

- (BOOL)isEqual:(id)object
{
	if (object == nil) return NO;
	if (![object isKindOfClass:[BinderKey class]]) return NO;

	BinderKey *b = (BinderKey *)object;
	return ((b.object == self.object) && [b.keyPath isEqual:self.keyPath]);
}

@end

Now that we have our binder, we can then bind our ViewModel to our view label in our ViewController:

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Load the contents from our server
	 */

	self.balance.text = @"";
	self.change.text = @"";
	self.viewModel = [[ViewModel alloc] init];

	self.binder = [[Binder alloc] init];
	[self.binder bindKeyPath:@"balance" ofObject:self.viewModel withCallback:^{
		double val = self.viewModel.balance;
		self.balance.text = [NSString stringWithFormat:@"%.2f",val];
	}];
}

Our new overall ViewController then becomes:

#import "ViewController.h"
#import "ViewModel.h"
#import "Binder.h"

@interface ViewController () <UITextFieldDelegate>
@property (weak, nonatomic) IBOutlet UILabel *balance;
@property (weak, nonatomic) IBOutlet UITextField *change;
@property (strong, nonatomic) ViewModel *viewModel;
@property (strong, nonatomic) Binder *binder;
@end

@implementation ViewController

- (void)viewDidLoad
{
	[super viewDidLoad];

	/*
	 *	Load the contents from our server
	 */

	self.balance.text = @"";
	self.change.text = @"";
	self.viewModel = [[ViewModel alloc] init];

	self.binder = [[Binder alloc] init];
	[self.binder bindKeyPath:@"balance" ofObject:self.viewModel withCallback:^{
		double val = self.viewModel.balance;
		self.balance.text = [NSString stringWithFormat:@"%.2f",val];
	}];
}

- (BOOL)textFieldShouldReturn:(UITextField *)textField
{
	double change = self.change.text.doubleValue;

	/*
	 *	Check to make sure our balance has been properly adjusted
	 */

	NSInteger err = [self.viewModel updateBalance:change];
	if (err == UPDATEERROR_ILLEGAL) {
		/*
		 *	This would draw to zero. Alert the user rather than make the
		 *	round trip to the server
		 */

		UIAlertController *c = [UIAlertController alertControllerWithTitle:@"Illegal value" message:@"You may not reset the balance below zero" preferredStyle:UIAlertControllerStyleAlert];

		UIAlertAction *action = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:nil];
		[c addAction:action];

		[self presentViewController:c animated:YES completion:nil];
	} else if (err == UPDATEERROR_OK) {
		self.change.text = @"";
	}

	return NO;
}

@end

And if we compile all of this, it works correctly.

Question: is this simpler?

Empirically our ViewController is doing nearly as much work. The only two things we have removed from the ViewController is one instance of loading the balance label from the -textFieldShouldReturn: message, and the business logic of validating prior to a network call of making sure our balance does not go below zero.

We’ve also added a new Binder object which is handling binding fields with values in our view model–a binder object which takes a text string to describe the field within our view model. (This is inherently fragile; if we were to change the name of the field in our ViewModel from ‘balance’ to something else, we would have no indications at compile time that something went wrong.)

We did push the logic for validating the inputs for -updateBalance: to the ViewModel, thus removing business logic from our ViewController. But note that we could have pushed the same logic into the ServerModel class by doing the following:

First, alter the signature of ServerModel so that -updateBalance:callback: returns a preflight error:

#import <Foundation/Foundation.h>

// Possible updateBalance return codes
#define PREFLIGHTERROR_OK		0
#define PREFLIGHTERROR_ILLEGAL	1

@interface ServerModel : NSObject

+ (ServerModel *)shared;

- (void)obtainBalance:(void (^)(NSInteger bal))callback;
- (NSInteger)updateBalance:(NSInteger)update callback:(void (^)(NSInteger bal))callback;

@end

Second, we change our server model class by adding the preflight checking to the server call:

- (NSInteger)updateBalance:(NSInteger)update callback:(void (^)(NSInteger bal))callback
{
	// Check against our cached value
	if (self.balance - update < 0) {
		return PREFLIGHTERROR_ILLEGAL;
	}

	// Server magic goes here

	return PREFLIGHTERROR_OK;
}

After all, the original papers describing the Model-View-Controller paradigm suggested the Model was not just simple database access code or simple network access code, but would also contain all of the business logic and data integrity checking logic as well. (That way, a second screen which needs to update the balance doesn’t need to duplicate business logic, as MVVM would have us do, and as our (admittedly) poorly written original ViewController class was doing.)

And note that our duplicate code that loads the balance label in the original example could have refactored that into a single method. No point in writing the same code twice, right?

So question: is our MVVM example simpler?

Well, we had to create two new classes for binding, but presumably they could be reused throughout our code. However, the binding code separates the functionality of refreshing the contents of our label from the process of asking for our label to be refreshed. This creates a sort of “spaghetti code” flow, where the result of an action is located in a completely different place from the logic which requested the action.

Our ViewController was not really made any simpler. (Sure, the business logic was located into a separate ViewModel, but given the MVVM model presumes one ViewModel per ViewController, we still have the problem that business logic can be shotgunned throughout our code.)

Our ViewModel certainly separated the concerns of data management for a specific view from the concern of managing the user interface–but then, as I argue above, that logic would better belong in a model which (as originally conceived in the 1980’s) contains all the integrity checks and doesn’t just contain data.

And by the simple metric of line count, we expanded one class which contained 66 lines of code to 96 total lines of code, expanding our source kit by 45%, as well as hiding the logic of tying in our view to a separate binder class. (And that 45% growth does not count our binding support classes.)

I do not intend to rag on MVVM. Every tool in the toolkit is useful, and it’s important to know about those tools, know how to use them, and know their limitations.

And you really should know MVVM, because it has become all the rage. Software architects may demand that you maintain MVVM code or convert code to MVVM, and knowing how to do that is important.

But in the above case, I have to say the answer to our question “is it simpler”, in all honestly, is no:

1. We increased the line count.
2. We altered the flow of code so that the response to an action (updating the balance label) is divorced from the process that requests the action.
3. We have not solved the problem of business logic shot-gunned through our code. All we’ve done is moved business logic into a ViewModel. Note MVVM suggests a one-to-one association between the View (ViewController) and the ViewModel classes. This means by definition business logic in a ViewModel class will necessarily be duplicated across multiple screens rather than placed into a single location.
4. And we haven’t even really solved the problem of unit tests completely, as one major source of problems (the constant string used to specify the field name in ViewModel) is in code (the ViewController) which would not be tested by hooking a unit test to our ViewModel.

I know I may be attacking a sacred cow here. After all, there are plenty of people who are convinced that MVVM is “worth it” even if it makes code more complicated.

And again, I’m not attacking MVVM; it’s just an easy example. There are plenty of other examples involving “code refactoring” which attempts to “generalize” a design pattern without considering other possible ways of approaching the problem. For example, see my essay on factorial code written in Java.

But do we need to make things more complicated in the name of an architectural ideal when, in a specific case, it is not called for?

Isn’t the world already full of too much complicated stuff to add to it ourselves?

First, a quick definition.

A design pattern is a fixed solution to a given software problem. For example, a singleton pattern is generally written as a single method which returns the single object, constructing it if it does not currently exist.

In the below I’m using the phrase programming idiom in the same sense we use the phrase to describe language: a more general way of expressing ourselves in code that conveys an idea or a feature. Unlike a design pattern, a programming idiom is not as rigid, but serves more as an organizing principle combined with a way of expressing ourselves in code.

Idioms may have a variety of ways of being expressed. And some “design patterns” are really programming idioms; ways to express ourselves in code that may have more than one possible solution.

(The word “idiom” may not be the best term, but I needed something to contrast from design pattern–which is used in a far more rigid way by most developers, and programming style, which seems to encompass something far more general. An idiom may express a way you solve a particular problem for a particular program which may not be repeatable or represent a general solution. How you clump statements into functions, for example.)

With that said, here are 12 principles for good idioms–good ways to reorganize or simplify your code. (With all due respect to Dieter Rams’ ten principles for good design.)

1. An idiom should make your code simpler.

There are many ways to express the same idea in code, and unnecessary complexity just makes life harder on everyone.

2. An idiom should make your code concise.

All too often I have seen people use a design pattern or refactor code, but fail to collapse parts of the code which are repeated. A good idiom (such as a mechanism for making an API call) is one which collapses the repeated statements into a common method somewhere else.

3. An idiom should make the intent of your code clear.

This goes hand-in-hand with making your code concise, but also means you should not hide functionality in implicit side effects or move important functionality elsewhere where it is difficult to discover.

4. An idiom should make your code easier to read.

Readability of software extends beyond just making your intent clear. It also has to do with how you name your variables, how you format the code on the page, common naming conventions you may adopt in your code.

5. An idiom should be unobtrusive and not clever.

All too often we see programmers trying to make their code “concise” by making clever use of language constructs which obscure the meaning of their code. But software should not be a way we tell the world how smart we are or how clever we are in using (or abusing) certain language features. Instead, we should be restrained in how we use the tools we have at expressing the ideas we are trying to express.

6. An idiom should help isolate functionality into well defined components.

A lot of software out there looks like it has been cut-and-pasted all over the source kit. Sometimes this cannot be helped thanks to constraints in time, but ideally all the software for a particular purpose should be kept together so it can be understood as a single component, and so it can be maintained as a single unit.

7. An idiom should stick to language features which make code obvious.

Some computer languages have interesting features which can help us solve some very difficult problems in a concise manner. Do not use them unless you absolutely have to.

In one case I saw code which used Objective C “swizzling”, which allows a reference to a method to be swapped in the background with a replacement method, to extend the functionality of UITableView on iOS. With the next version of the operating system, the software crashed hard. Worse, because the swizzling process affected the underlying class declaration, this created a difficult to diagnose crashing problem that affected all parts of the application. This, despite the fact that the functionality added by swizzling was only used on one screen.

The solution? Create a subclass and override the methods that needed to be altered using inheritance. Not as sexy, but the intent of the code was more clearly expressed, and the changes caused by swizzling was then isolated to a single screen where they could be dealt with in a more reasonable manner.

8. An idiom should keep related functionality together in the same place.

Beyond just building well-defined components, we should strive to keep code related in functionality together, as well as code that is related in execution. Components should be built out of methods grouped by related functionality; components should be assembled into collections of classes which represent similar functionality or similar features. And as much as possible the code which is executed as a consequence of an operation should be kept together with code that performs an operation.

9. An idiom should be debuggable.

The built-in debugger in most IDEs are fantastic; they allow us to quickly make changes and see the results of our changes in a few seconds. And yet we see developers unintentionally sabotage their IDEs with code that uses features (such as the Java proxy mechanism or Java introspection combined with serialization) which make it impossible for us to know how the code flows, where data for an operation was obtained, or the result of a computation.

And in the process of being clever, that developer has effectively tied our hands behind our back, making it impossible for us to determine how the code works by seeing it in action.

10. An idiom should not require extraordinary steps to compile or break the existing development tool set.

I once worked on a project written in Java, written using the Eclipse IDE, which could not be compiled within Eclipse, but only with the Unix make utility. Worse, it took an hour to build the product. Needless to say this severely limited developer productivity.

And while this is an extreme example, all too often I’ve seen solutions which require the setup of special tools or which require the incorporation of build steps which may break the IDE.

Sometimes, of course, you have no choice. Objective C and C++ both started as preprocessor programs which rewrote the code into C. But this should be far more rare than it is in practice.

11. An idiom should only implement the functionality needed.

Too often we get it in our minds to write the “general solution”, or the “extensible solution”, or solve problems we don’t have in the event we may need to solve them later. Don’t do this. Instead, just solve the problem you have today; if your code is well organized you’ll have plenty of time tomorrow to solve tomorrow’s problems.

More importantly, you cannot know what problems you’ll have tomorrow. So all you are doing is adding additional complexity and creating additional work for yourself when you don’t need to.

12. An idiom should allow a new developer to understand the code.

The way you organize your code should make it easier for a new developer to understand not only the intent of your code, but also understand how it works, how data flows through your program—and more importantly, where to fix bugs and add new features in a way which is natural to the code base.

Make everything as simple as possible, but not simpler. – Albert Einstein.