This is a continuation of the blog post 12 Principles of Good Software Design.
8. An idiom should keep related functionality together in the same place.
In a 1968 edition of The Communications of the ACM, Edsger Dijkstra published a paper A Case against the GO TO Statement, where he made the argument that the GO TO (or GOTO) statement popular in many programming languages at the time was harmful.
The unbridled use of the go to statement has an immediate consequence that it becomes terribly hard to find a meaningful set of coordinates in which to describe the process progress.
That is, the GOTO statement allows us to construct “spaghetti code”, it is impossible to know by examining a small segment of code how that chunk of code would be executed, when it would be executed or under what conditions it would be executed. Worse, a chunk of code towards the end of a function may be logically related to the code at the start of a function–but we have no way to understand this by casual inspection.
To give a concrete example of what this means, suppose we have the following code in BASIC:
SUB printlist LET x = 0 LET len = 10 10: IF x >= len GOTO 20 PRINT x x = x + 1 GOTO 10 20: END SUB
Compare and contrast to the following written in C:
int len = 10; for (int x = 0; x < len; ++x) { print(x); }
Both of these do the exact same thing: we iterate x from 0 to 10, printing x each time through the loop.
In C our for loop keeps the related functionality (iterating through the loop) together in a single line, which makes the intent of our code obvious.
Our BASIC example using GOTO statements, however, scatters the code which implements our loop.
Coloring the elements of our C statement to show our index being initialized, updated and tested against the limit, we see:
int len = 10; for (int x = 0; x < len; ++x) { print(x); }
Now look at where the corresponding elements are in our BASIC statement:
SUB printlist LET x = 0 LET len = 10 10: IF x >= len GOTO 20 PRINT x x = x + 1 GOTO 10 20: END SUB
Now this is a trivial case, but imagine if you have two embedded for loops combined with a rather complex inner loop. It would be nearly impossible using GOTO to understand what is going on–because we have violated the principle of keeping related functionality together in the same place.
The idea that GOTOs are considered dangerous stems from the idea that it makes it harder for us to read the software we’ve written. Of course GOTO statements do not necessarily lead to spaghetti code, but unfortunately it makes it incredibly easy, and that obfuscation makes code maintenance and fixing bugs difficult to impossible.
It’s not just GOTO statements which can lead to spaghetti code. In fact, it is poor code hygiene which causes the code for a given feature to be scattered across multiple locations in our code–and even across multiple classes or modules as well as in multiple disparate places in the same source module. GOTO may make good code hygiene more difficult, but simply ignoring GOTO statements without understanding code hygiene is just as harmful.
There are many ways in which functionality expresses itself in code, and many way where we can practice good code hygiene by keeping related functionality together.
Functionality can be related by side effect or by operation; for example, a component may need to initialize a number of other subcomponents, and it would be desirable (as much as possible) to show each component being completely initialized before the next component, rather than mixing initialization code in a jumble.
A previous example demonstrated one of these. Suppose we have a view controller which is initializing multiple buttons by setting their appearance:
- (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; }
Notice the spaghetti of our initialization code; it’s unclear which button is being initialized in what way. By simply reordering the statements by related functionality we can understand better what is going on:
- (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; }
Readability is enhanced by keeping related functionality together.
Functionality can be related by feature: a custom view with drawing code would be easier to understand if all of the drawing code were kept together and perhaps ordered from the most specific to the most general.
For example, suppose we have a custom UIView which draws a series of four buttons, and send a message if one of the buttons is tapped on by the user.
The functionality of our custom UIView can be broken down into three general bits of functionality:
-
- Calculate the location of each of the buttons.
- Draw the individual buttons.
- Handle tap events by calling our callback.
With these features in mind we can build our custom view.
First, we need to calculate the location of our four buttons. We assume our buttons are all 44×44 pixels in size, laid side by side. The intrinsic size of our view is then 44 pixels tall by 44 * 4 = 176 pixels wide.
- (CGSize)intrinsicContentSize { return CGSizeMake(176, 44); } - (CGRect)calcButtonAtIndex:(NSInteger)index { return CGRectMake(44*index,0,44,44); }
The first returns our intrinsic size. The second calculates the rectangle for each “button” we’re drawing in our custom view.
The second feature involves drawing the contents. We need to draw each button in the rectangle we obtain from the -calcButtonAtIndex: method:
// From PaintCode file. - (void)drawCanvas1WithFrame: (CGRect)frame label: (NSString*)label { //// General Declarations CGContextRef context = UIGraphicsGetCurrentContext(); //// Rectangle Drawing CGRect rectangleRect = CGRectMake(CGRectGetMinX(frame) + 2.5, CGRectGetMinY(frame) + 4.5, CGRectGetWidth(frame) - 5, CGRectGetHeight(frame) - 9); UIBezierPath* rectanglePath = [UIBezierPath bezierPathWithRoundedRect: rectangleRect cornerRadius: 6]; [UIColor.lightGrayColor setStroke]; rectanglePath.lineWidth = 1; [rectanglePath stroke]; NSMutableParagraphStyle* rectangleStyle = [NSMutableParagraphStyle new]; rectangleStyle.alignment = NSTextAlignmentCenter; NSDictionary* rectangleFontAttributes = @{NSFontAttributeName: [UIFont systemFontOfSize: UIFont.buttonFontSize], NSForegroundColorAttributeName: UIColor.blackColor, NSParagraphStyleAttributeName: rectangleStyle}; CGFloat rectangleTextHeight = [label boundingRectWithSize: CGSizeMake(rectangleRect.size.width, INFINITY) options: NSStringDrawingUsesLineFragmentOrigin attributes: rectangleFontAttributes context: nil].size.height; CGContextSaveGState(context); CGContextClipToRect(context, rectangleRect); [label drawInRect: CGRectMake(CGRectGetMinX(rectangleRect), CGRectGetMinY(rectangleRect) + (CGRectGetHeight(rectangleRect) - rectangleTextHeight) / 2, CGRectGetWidth(rectangleRect), rectangleTextHeight) withAttributes: rectangleFontAttributes]; CGContextRestoreGState(context); } - (void)drawRect:(CGRect)rect { for (NSInteger i = 0; i < 4; ++i) { NSString *label = [NSString stringWithFormat:@"%d",(int)i]; CGRect r = [self calcButtonAtIndex:i]; [self drawButtonWithFrame:r label:label]; } }
Note the button drawing itself was created using the PaintCode app, which is one of those powerful tools you really need if you are creating iOS software. It can automate the creation of custom drawing code incredibly simple. But because the style of the code doesn’t match the style of the software we’ve built elsewhere, it’s worth noting where the code came from.
The third feature involves testing for tap events. Because we’re calculating the rectangle in a separate routine, our hit detection code becomes very simple:
- (void)touchesBegan:(NSSet *)touches withEvent:(UIEvent *)event { } - (void)touchesMoved:(NSSet *)touches withEvent:(UIEvent *)event { } - (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event { } - (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event { UITouch *touch = [touches anyObject]; CGPoint where = [touch locationInView:self]; for (NSInteger i = 0; i < 4; ++i) { CGRect r = [self calcButtonAtIndex:i]; if (CGRectContainsPoint(r, where)) { if (self.tapCallback) { self.tapCallback(i); } } } }
Note: if you need to declare one of the tap methods you should declare all four. The iOS tap hit detection mechanism appears to get confused if you only declare one of these methods.
Putting together these three bits of functionality together in a single class, we get our class declaration:
#import <UIKit/UIKit.h> @interface CustomButtons : UIView @property (copy) void (^tapCallback)(NSInteger i); @end
And our custom view class:
#import "CustomButtons.h" @implementation CustomButtons #pragma mark - Layout calculation - (CGSize)intrinsicContentSize { return CGSizeMake(176, 44); } - (CGRect)calcButtonAtIndex:(NSInteger)index { return CGRectMake(44*index,0,44,44); } #pragma mark - Drawing Support - (void)drawButtonWithFrame: (CGRect)frame label: (NSString*)label { //// General Declarations CGContextRef context = UIGraphicsGetCurrentContext(); //// Rectangle Drawing CGRect rectangleRect = CGRectMake(CGRectGetMinX(frame) + 0.5, CGRectGetMinY(frame) + 0.5, CGRectGetWidth(frame) - 1, CGRectGetHeight(frame) - 1); UIBezierPath* rectanglePath = [UIBezierPath bezierPathWithRect: rectangleRect]; [UIColor.lightGrayColor setStroke]; rectanglePath.lineWidth = 1; [rectanglePath stroke]; NSMutableParagraphStyle* rectangleStyle = [NSMutableParagraphStyle new]; rectangleStyle.alignment = NSTextAlignmentCenter; NSDictionary* rectangleFontAttributes = @{NSFontAttributeName: [UIFont systemFontOfSize: UIFont.buttonFontSize], NSForegroundColorAttributeName: UIColor.blackColor, NSParagraphStyleAttributeName: rectangleStyle}; CGFloat rectangleTextHeight = [label boundingRectWithSize: CGSizeMake(rectangleRect.size.width, INFINITY) options: NSStringDrawingUsesLineFragmentOrigin attributes: rectangleFontAttributes context: nil].size.height; CGContextSaveGState(context); CGContextClipToRect(context, rectangleRect); [label drawInRect: CGRectMake(CGRectGetMinX(rectangleRect), CGRectGetMinY(rectangleRect) + (CGRectGetHeight(rectangleRect) - rectangleTextHeight) / 2, CGRectGetWidth(rectangleRect), rectangleTextHeight) withAttributes: rectangleFontAttributes]; CGContextRestoreGState(context); } - (void)drawRect:(CGRect)rect { for (NSInteger i = 0; i < 4; ++i) { NSString *label = [NSString stringWithFormat:@"%d",(int)i]; CGRect r = [self calcButtonAtIndex:i]; [self drawButtonWithFrame:r label:label]; } } #pragma mark - Touch Events - (void)touchesBegan:(NSSet *)touches withEvent:(UIEvent *)event { } - (void)touchesMoved:(NSSet *)touches withEvent:(UIEvent *)event { } - (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event { } - (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event { UITouch *touch = [touches anyObject]; CGPoint where = [touch locationInView:self]; for (NSInteger i = 0; i < 4; ++i) { CGRect r = [self calcButtonAtIndex:i]; if (CGRectContainsPoint(r, where)) { if (self.tapCallback) { self.tapCallback(i); } } } } @end
Because we’ve successfully grouped the functionality of our component, we can use the #pragma mark directive to help document the functionality of our component, allowing future developers to understand the pieces that make our component work.
Functionality may be related by collections of objects implementing an application feature: all of the components used to implement a language parser should be kept together. Java has the concept of a “package” which represents a collection of classes that implement a single feature, but we can achieve the same effect in Xcode for Objective C by organizing our project source kit into groups.
For example, suppose we need to build a parser which parses strings containing simple arithmetic equations, returning an abstract syntax tree that can be used later to evaluate the equations.
For our algorithm we could use the Shunting-yard algorithm, which allows us to properly group equations according to operator precedence. First, however, we need to group characters and letters into symbols, and for that we will use a Lexical analysis tool. Our lexical analysis tool could have the following declaration, taking an input string and providing functions for extracting the next token (either a variable, a number or an operator or parenthesis) for parsing.
#define LEX_VARIABLE 65536 #define LEX_NUMERIC 65537 @interface LexicalParser : NSObject - (id)initWithString:(NSString *)str; - (NSInteger)nextToken; - (NSString *)lastToken; - (void)pushBackToken; @end
Our methods allow us to get the next token–either a character (such as ‘+’ or ‘*’), or a token, a collection of characters representing a variable name or a number. If we returned a token, -lastToken returns a string representing the token. And -pushBackToken allows us to set a flag which effectively pushes back a single token.
On top of this class we would implement the parser, the shunting yard algorithm which allows us to convert the tokens from our lexical analysis parser into an abstract tree:
@interface EquationParser : NSObject - (id)initWithLexicalParser:(LexicalParser *)lex; - (AbstractSyntaxNode *)parse; @end
The abstract syntax tree is represented by a collection of nodes, with the format:
@interface AbstractSyntaxNode: NSObject @property (nonatomic, assign) BOOL leaf; // value // if leaf is true @property (nonatomic, copy) NSString *value; // if leaf is false @property (nonatomic, assign) NSInteger op; @property (nonatomic, strong) AbstractSyntaxNode *left; @property (nonatomic, strong) AbstractSyntaxNode *right; @end
After going through all this work to build these three interrelated classes used to implement a single feature, it’d be a shame if we were to organize our project as follows:
Why?
Because we have no way as we browse the project to understand that we have an equation parser that is built using three interrelated classes.
However, if we were to organize our project as follows:
Now we have a clue how our equation parser may work.
Functionality may be related by the sequence of execution statements; by using anonymous classes in Java or blocks in Objective C we can keep the code executed as a result of an asynchronous statement next to the code which triggers the asynchronous operation.
From a previous example, we can use blocks with an asynchronous operation in order to keep the code which triggers an operation together with the code that is called as a result of that operation.
So our API which makes a request to a remote server and returns a result has the following declaration:
@interface Server : NSObject + (Server *)shared; - (void)requestWithEndpoint:(NSString *)endpoint parameters:(NSDictionary *)params completionHandler:(void (^)(NSDictionary *data, NSURLResponse *response, NSError *error))callback; @end
This allows us to write our request as follows:
- (void)doOperation { int value1 = (do something here); NSDictionary *d = @{ @"arg1": @( value1 ) }; [[Server shared] requestWithEndpoint:@"apicall" parameters:d completionHandler:^(NSDictionary *data, NSURLResponse *response, NSError *error) { NSInteger retVal = [data[@"result"] integerValue]; (handle our result here) }]; }
Notice that not only is our code here concise; it is obviously making an API call to the endpoint “apicall” with a single parameter with an argument named “arg1”, but we can see immediately following it in the code how we handle our result.
Had we written our declaration differently, say with a response object passed in to our request:
@protocol ServerResponse - (void)completionWithData:(NSDictionary *)data response:(NSURLResponse *)response userData:(NSDictionary *)userData; @end @interface Server : NSObject + (Server *)shared; - (void)requestWithEndpoint:(NSString *)endpoint parameters:(NSDictionary *)params userData:(NSDictionary *)userData response:(id)response; @end
Then our call may seem simpler:
- (void)doOperation { int value1 = (do something here); NSDictionary *d = @{ @"arg1": @( value1 ) }; [[Server shared] requestWithEndpoint:@"apicall" parameters:d userData:nil response:self]; }
But our call does not reveal to us what action is taken when we get a response. Worse, we don’t even have an obvious way of knowing where the response will arrive in our class.
Good code hygiene requires far more of us than simply not using GOTO statements. It requires us to consider how our code holds together at each level of the implementation, from individual statements to collections of classes, and how we handle responses to asynchronous messages.
GOTO statements are bad because it makes it easy to write spaghetti code; it makes it too easy for us to scatter related functions all throughout the source kit, without any clue as to where the code lives for a given set of features. But bad code hygiene–failing to keep functionality related by feature, by function or by code execution–can lead to spaghetti code even without the use of a single GOTO statement, even in a computer language otherwise designed to help make code better organized.