The Fifth Principle: An idiom should be unobtrusive and not clever.

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:

Bind1

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:

Screen1

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)

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s