Stop the Insanity

(Originally posted 2003.Jun.06 Fri; links may have expired.)

Some people very experienced in software development looked at my Extract Method refactoring example and wondered what was so unusual about doing that. One asked "Isn't this just obvious, and what any sane person would do?" Apparently not.

There are tons of "insane", unrefactored code in companies everywhere. Even in my own current XP-ish project, there are developers who sometimes write (or more likely copy-paste-modify) long methods with duplicate code, and they have more than five years of experience developing software.

For many people, it seems like their tolerance level for hard-to-read, hard-to-maintain code is high, possibly because their high intelligence lets them sort things out in their minds - at least when they're familiar with the code. If only they would sort it out in the code itself.

I imagine those "tolerant" people get irritated by us "intolerant" Extreme Programmers, who want the code to be so easy to read and enhance that it looks like it was easy to write. There's an old joke in programmers circles - "If it was hard to write, it should be hard to read" - that is unfortunately too true.

If you really want to get insane code - long methods, lots of duplication - measure your programmer's productivity by counting lines of code. You'll get what you measure.

Code Reviews versus Pair Programming

(Originally posted 2003.Jun.09 Mon; links may have expired.)

On the XP mailing list today, Ron Jeffries responds to a question about code reviews:

Frankly, code reviews are so much worse than pair programming that a dose of them would make me fly to pair. Let's see if we can replicate my experience.

Here's one path through a network of a million decision points:

To do code reviews, everyone has to read the code beforehand, unless you're
doing a walkthrough, see below. I'd ask everyone to come together
physically to the review. Then I'd ask them to report truthfully how much
time they spent reviewing the code. Early on, I would report truthfully
that I had spent zero or very little time, in hopes of getting others to
admit the truth. When they admit the truth, I'd dismiss the meeting and
reschedule it.

Dismiss and reschedule is the appropriate response to lack of preparation, according to Freedman and Wienberg.

Then, after a while, the only alternative is a walkthrough, since no one is
preparing effectively for the review. So we do walkthroughs for a while.
They are intensely boring, and few people stay involved. Note in your mind
the people who are present but not involved. At the end of the session,
say, holding your hand up, "Who else had a real problem staying engaged
with this walkthrough?" If there's honesty in the room, hands will go up.
Prompting may be necessary. Then: "Any ideas?"

Surely someone will think of "doing this in smaller groups or one on one".
Try it. Ask the team whether "we should empower the one-on-one folks to
change the code, and under what circumstances." Don't mention that this is
pair programming.

Try an experiment. You're "interested in collaborative programming".
Interested parties should come to the room to help. On the screen, start
writing a program. Ask for help with it, get the room to pair with you. Get
stuck (no need to fake this if you are me). Someone will start telling you
what to do. Don't get it (no need to fake this either). Get them to come up
and do it ... grabbing the chair that is accidentally beside you, while you
move over.

Note that reviews often find things. Observe how many of them are resisted
by the original programmer, or are "too much trouble to fix now".

Build a few BVCs [Big Visible Charts] relating to time spent prepping, in the meetings, number of useful suggestions (by person if you can do it without problems), number of changes made in response to suggestions, ...

Code reviews are intensely painful, in my experience, and we were trained
by Freedman himself. There will be no need to set them up to be perceived
that way, though it will take honesty among the group to express it. After
doing enough code reviews, which take way more than half the groups' time
by the way, a team who has heard of pair programming should be begging to
pair. About all you have to do is make sure that no one treats the review
session as nap time, and that you are early in recognizing the people who think it's a waste of time. Because they're right.

I have done reviews on a project that was building software for a medical device. We prepared for the code review (and also reviewed unit tests). We found issues (like not enough unit tests for specific parts of the code). The programmer corrected issues after the review, and had a second review (a walkthrough with the coder and one reviewer) to confirm that the issues were corrected. We signed off pieces of paper to have the paper trail that the FDA would expect. It wasn't the most exciting work, but we were one of the best teams, people-wise, that I've ever been privileged to work with. Truly respecting each other and working together very well.

Coding and reviewing this way was very slow - for eight person-hours of programming, expect at least a half an hour of preparation (coder and three reviewers - so two person-hours of preparation), one hour of review (times four people - so four person-hours of review), one person-hour correcting issues, and another half-hour of walkthrough (times two people - one person-hour). So for 8 person-hours of programming, there was another 8 person-hours of overhead: reviews, correcting, and walkthrough.

As is often said, "basic XP" isn't intended for safety-critical applications. But I could see the following working in that environment: pair-programming and test-driven development for four hours (eight person-hours), and a one-hour walk-through by two people who didn't do the pair-programming (two person-hours). I would expect very few issues to be found in the walkthrough that hadn't already been found (or avoided) and corrected in the pair programming, and therefore there would be much less need for correcting issues. So 8 person-hours of (pair) programming would have an overhead of 2 person-hours of walkthroughs.

While I was working on the medical device project, we didn't have acceptance testers nor automated acceptance tests. If we had been doing XP, we would have had better testing -- better unit tests, and real acceptance tests. I didn't stay on that project through completion - when our (well-liked but not very political) manager was removed from the project, and our team leader quit, almost everyone else on the team quit too. I was one of the last ones to leave, accepting an offer from Apple Computer, Inc. and moving to California. I heard the the project eventually finished, with half the man-power and twice the planned schedule. Very likely the acceptance testing was done at the end of that project.

Selling the Invisible

Originally posted 2003.May.31 Sat; links may have expired.

I found the book Selling the Invisible by Harry Beckwith to be a very interesting and inspiring read. Those of us working in software development deal with the invisible all the time. While some of us may write shrink-wrap software, and can point to the box as proof that our product exists, even that product can be mostly invisible: anti-virus software, for example, should be invisible until it tells you it found and eliminated a virus (and maybe even then).

Selling the Invisible is about marketing and a lot more: some of the best marketing comes from having a good product or service, and happy clients or buyers. It comes from maintaining good relationships. I recommend this book. It gave me an idea for my company that could be profitable, and I've only read the book once through so far.

"Management" is mostly invisible - the worse management is, the more visible it is. The best managers (I'm told) often seem like they are neither busy nor doing "any work," because they've done a lot of work to keep potential problems from turning into actual problems. Managers could do with reading this book, considering their bosses, peers, customers, suppliers, and direct reports as "customers", all of whom to maintain good relationships with.

Software development methods can be invisible too. Particularly agile methods that don't produce many artifacts other than the software itself. The recommended "big visible charts" (in particular the scrum burn-down chart adapted for XP) help make the invisible stuff visible, and help keep a good relationship with the customer.

In fact, I thought Kent Beck wrote a message in the IXP mailing list that sounded like the "positioning statement" exercise of Selling the Invisible, but now I can't find it. I'll attempt to do the exercise here, using me as an XP coach/project leader. The positioning statement answers the questions of Who, what, for whom, what need, against whom, what's different, and what benefit.

Keith Ray is a coach and project leader for small to medium-sized projects (up to 10 programmers) which need to ship good quality software, quickly and/or frequently, whether the desired features are vaguely defined, changing, or very specific. Unlike many in this industry, Keith Ray has been developing software for over 15 years, and has participated in many successful projects in several product areas, creating and shipping good software at a predictable pace. Keith Ray uses good, context-specific, practices to enable you to get the important features you want as soon as possible.

You notice I don't say "XP" or "agile" in this statement - people are looking for solutions to their problems, and don't care about the techniques I use (I hope).  What's your positioning statement?

Swift - initializing CGFloat values

import UIKit

var red: Int = 12

var newRed1 = red/255
// newRed1 is Int with value 0

// var newRed2 = red/255.0
// Error: Cannot invoke '/' with an argument list of type '(@lvalue Int,FloatLiteralConvertible)'

var newRed3 = CGFloat(red)/255.0
// newRed3 is CGFloat with value 0.0470588235294118

var newRed4 = CGFloat(red)/255
// newRed4 is CGFloat with value 0.0470588235294118

var newRed5 = CGFloat(red/255)
// newRed5 is CGFloat with value 0.0

// This one is surprising:
// var newRed6 = CGFloat(red/255.0)
// Error: "Could not find an overload for 'init' that accepts the supplied arguments"

myGraph is free for Christmas

myGraph by Sizeography, the app that prints graph paper, is free for Christmas and from now (Dec 23, 2014) until the end of the year. Free!

Besides printing standard kinds of grids and graph paper, you can customize the colors, spacing, and line-width. Do you want green and red? You can have it.

Check it out!

Readable Code

Recently, I needed to change a scalar variable into a stack, as I modified my code to handle a new feature. Here's what I did.

First, I looked to see if the libraries I had access to contained a Stack class. (Apple's Foundation framework, etc.) I didn't find one. Nor did I find 'push' and 'pop' methods.

I could have changed the variable to an NSMutableArray and used these two methods instead of 'push' and 'pop':

- (void)addObject:(id)anObject;  // adds to the end of the array
- (void)removeLastObject;  // removes from the end of the array

Functionally, those two method would serve as 'push' and 'pop', but it would be easy for future-me, or another member of the developer team, to forget that the object is supposed to be used like a stack.

One option would be to add methods to the NSMutableArray class via a class category. That would allow my code to call 'push' or 'pop' but still doesn't clearly express my intent for this variable (which would still be declared as an NSMutableArray.

Since a basic stack is very easy to construct, I built one very quickly with just the API that I wanted. This is a common enough "demo" of Test-Driven-Development that I'm not going to reproduce it hear.

One thing did come up: I will sometimes want to display the contents of the entire stack, and I don't want to push/pop everything to do so. So I added another stack test and another method to my stack class.  Since I know the stack will be small (less than 30 objects), I return a copy the internal array storage, rather than returning an enumerator for the caller to access elements one-at-a-time.

-(void) testContents

{

    [stack push: @"a"];

    [stack push: @"b"];

    [stack push: @"c"];

    AssertEqual(3, [stack count]);

    NSArray *arr = [stack contents];

    AssertEqual(3, [arr count]); // calling contents doesn't pop.

    AssertObjectsEqual(@"a", arr[0]); // oldest

    AssertObjectsEqual(@"b", arr[1]);

    AssertObjectsEqual(@"c", arr[2]); // newest - top of stack

}

...

-(NSArray *) contents

{

    return [_storage copy];

}

...

Getting the data in this order (from the bottom of stack to the top of stack, is precisely what I want, since I be displaying the contents horizontally in the UI. 

But... I think 'contents' isn't clear enough. So I renamed the method:

-(NSArray *) contentsBottomUp

{

    return [_storage copy];

}

And, for symmetry, I added another method (and test) to return stack contents top-down instead of bottom-up. While not strictly needed for the current UI. I might want to display the stack contents vertically, and then I'd want the display the top first, and the bottom last.

-(NSArray *) contentsTopDown;

Since I didn't find a 'reverse' method in the Foundation framework either, I needed to implement a way to reverse the contents being returned from 'contentsTopDown'. While looking for examples of code using Apple's 'Fast Enumeration' implementation, I found this one-liner in NSHipster's blog:

-(NSArray *) contentsTopDown;

{

    return _storage.reverseObjectEnumerator.allObjects;

}

The end.

Mocking

The question: "I encountered an issue when I try to mock 3rd-party library functions.  The MOCK_METHOD_i macro supports i = 1 to 10 arguments, but 3rd-party library has functions with 11 arguments. Should I alter the scripts that build the MOCK_METHOD_i macros?"

My answer:

Yes and no.

Yes: so go ahead and rebuild the mocking framework by setting "n = 12 or so" in the script the generates the MOCK_METHOD_i macros. Make the tools do the work you want them to do.


No: it's often better to only mock code that you own. If you make that a rule, then what you have to mock is code that you wrote, code which hides and (hopefully) simplifies access to the external code. Your own code will likely NOT need 11 arguments, since you can hide unnecessary parts of the external code. 

There's a nice wrap-up of this concept here: http://www.davesquared.net/2011/04/dont-mock-types-you-dont-own.html

Using test doubles for types we don’t own can end up with fragile tests that don’t actually test much of value, or can even compromise our design and the effectiveness of the abstractions we use. Even when mocking a library we know really well we can end up with compromised abstractions and fragile tests due to relying on implementation details or assumptions based on previous versions of the library.

iPhone and iPad Models and Screen Resolutions

iPhone and iPad models and screen resolutions in the context of Apple's Xcode launch image xcassets view.

I and Me

If you're wondering whether to say "Bob and I" or "Bob and me," here's the trick: Take "Bob and" out of the sentence, correct the number implied by other words, if necessary, and judge whether the sentence sounds right.


Example 1: "Bob and I just want the best for you."

Drop Bob: "I just want the best for you."

The rewritten sentence sounds right, so "Bob and I" is correct in this example. The sentence should be, "Bob and I just want the best for you."


Example 2: "That award belongs to Bob and I."

Drop Bob: "That award belongs to I."

That doesn't sound right. So this example should be "That award belongs to Bob and me."

And you can test that again by changing "I" to "me" and leaving out Bob. "That award belongs to me." (But don't say that to Bob!)


Example 3: "We want to create the same experience you would have if you had called both Bob and I into your office for a meeting."

Drop Bob: "I want to create the same experience you would have if you had called I into your office  for a meeting."

That doesn't sound right. Let's try "me."  "I want to create the same experience you would have if you called me to your office  for a meeting." Better.

So, the better sentence is: "We want to create the same experience you would have if you had called both Bob and me into your office for a meeting."


That's it. Those of us who speak English as a native tongue, already know the unspoken rules of grammar; we just get confused when we try to follow the artificial rules we were taught in schools or by well-meaning friends.

Two Weird 'Tricks' For Testing Graphical Code

Before I do a bunch of "Extract Method" refactorings, I looked over the code and wrote this comment:

    // creates pdf context.

    // computes rowCount and columnCount from paper_size

    //    and major_grid_distance.

    // computes margin sizes (and thus origin.)

    // draws graph paper into context.

    // close context.

    -(void) drawMajorMinorGridAt: (CGPoint) origin

                     andRowCount: (int32_t) numberOfRows

                  andColumnCount: (int32_t) numberOfColumns

                       inContext: (CGContextRef) context;

What the extract methods will do, is let me draw while not in a pdf context. Code-reuse: yay!

Another way to achieve the same goal: copy & paste. While copy & paste may be quicker,  it bulks up the code and potentially doubles the number of defects that may "bug" us in the future. Technical debt.

One could argue that planning ahead would have created a better design. But... I didn't need to draw while not in a pdf context, a half-year ago.

One reason this code is so bad, is that I didn't exactly test-drive it. I ran some calculations, wrote the code, and looked at the output. In some early iterations, it produced grids with lines poking too far through other lines: I had to reduce the length of some lines, by the thickness of other lines. Similar glitches provoked a few other changes. And then it was done. I haven't touched the code in quite a while. 

Could I write some automated tests for this code? Yes. 

I could save some manually-verified pdf output as "Golden Data" and do file comparison in tests that are supposed to produce the same output. I have done something like this before. The only time the test failed in my project (without my changing code to cause the failure) was when a graphics library that my code depended on changed. 

So... few "false" failures. Drawbacks: slow tests, manual verification needed if the test fails. And manual intervention to re-create the "Golden Data" when the code is fixed or the failure is identified as not a bug. I also had to write a custom file-comparison routine in order to ignore parts of the file that change all the time (a time-stamp in the file itself.)

You do not want to do "Golden Data" test data when false failures are frequent. (Or any kind of manual intervention is frequent.)

Mocking. Yes, I could mock out the drawing code. It's a bit difficult because the drawing code below this Objective-C class consists of C functions. I would have to create a "seam" to allow compile-time, link-time, or run-time switching out of the drawing library. 

Mocking can create fragile tests. Since mocking works by verifying that the code-under-test executes an expected sequence of function-calls, and checks that the parameters passed into those functions have expected values, any change to the order of calls or the parameters passed in will cause the test to fail. This depends on how picky you configure the mock objects to be.

With some work, depending on the mocking framework, you could ignore the exact sequence of function calls, but still check that all of the calls have been made. Make the mocks too "loose" and then you're not testing anything. Make the mocks too strict, and you not only open yourself up to the possibility of frequent false failures, you also inhibit refactoring the drawing code.

There is also the danger that the functions you're avoiding calling will someday change their behavior, but the mock functions don't reflect that change. Instead of false test failures, you get false test successes. Some redundancy in testing helps avoid this problem: have some tests using mocks, have other tests using the real functions and some other form of verification, like Golden Data.)

Learn about Golden Data, mocks, and other xUnit Test Patterns here: xUnit Test Patterns: Refactoring Test Code

Test-drive safely, everyone!

When Your Brain Freezes Up

Some time ago, I was part of team for a trivia contest. There was no internet or computer access at that location, so no chance of cheating. I took the lead on some of the questions and got them wrong!

Why did that happen? I expected the questions to be hard. I expected the trivia questions to have non-obvious answers. It turned out that the obvious answers were the correct ones.

Go to this you-tube "video" (it's audio only) where a radio disk-jockey is posing a question to someone they randomly called. If the person they called gets the question right, they could win a motorcycle worth around $8,000 (US).

DJ: We've got to ask you a question before we put your name in the barrel. Spell AC/DC.
Mark: AD AC
DJ: I'm going to ask you again.
Mark: Hang on.
DJ: How do you spell AC/DC?
Mark: A… D… A… C

There's some overlapping dialog, and we hear Mark say "ACDC" in that noise. But his answer when asked a third time is still ADAC, until they call him an idiot and tell him it's AC/DC!

It could be this guy had other issues, but under stress, our body is preparing for flight or fight. The neocortex is only a recent invention. "Flight or fight" has been around for billions of years.

Function Pointer Syntax Cheat-Sheet-By-Example

#include <stdio.h>

// Using * to indicate the parameter is a pointer to a function.

void functionTakingFunctionPointerOldStyle(int (*pf)(int))
{
    printf("\nold style pf = %p\n", pf);
    if (pf != NULL)
    {
        int x = (*pf)(12); // Using * to deref function pointer.
        printf("pf(12) = %d\n", x);
    }
}

// We don't need * to indicate that a parameter is a function pointer.

void functionTakingFunctionPointerNewStyle(int pf(int))
{
    printf("\nnew style pf = %p\n", pf);
    if (pf != NULL)
    {
        int x = pf(12); // Don't need * to deref function pointer, either.
        printf("pf(12) = %d\n", x);
    }
}

// Blocks. Like old-style function pointer, using ^ instead of *.

void functionTakingBlock(int (^pf)(int)) // like old-style function pointer.
{
    printf("\nblock pf = %p\n", pf);
    if (pf != NULL)
    {
        int x = pf(12); // Same syntax as calling function or function-pointer.
        printf("pf(12) = %d\n", x);
    }
}

/*

If we use a typedef, then the syntax for parameter declaration is simple:

     typedef int (*funcPtr)(int);

     void functionTakingFunctionPointer(funcPtr fp);

 But now we need to remember how to declare the function-pointer typedef!

 This is how I create a typedef for a function pointer type:

 1. Start with the function declaration itself.

     int functionTakingIntReturningInt(int i);

 2. Remove parameter names.

     int functionTakingIntReturningInt(int);

 3. Change the function name to a name appropriate for this function-pointer type, maybe suffixed with "FuncPtr" if you can't think of something more meaningful. Don't suffix "Type" to the name—like "FuncPtrType"—it makes you look like a newby. (I stopped doing that last week.)

     int funcPtr(int);

 4. Wrap name with (* and )

     int (*funcPtr)(int);

 5. Add typedef

     typedef int (*funcPtr)(int);

 6. Profit!

*/

int functionTakingIntReturningInt(int i)
{
    return i + 1;
}

int main(int argc, char *argv[]) 
{
    // "old" style C code - using "&" take address of the function.
    functionTakingFunctionPointerOldStyle( NULL );
    functionTakingFunctionPointerOldStyle( & FuncionTakingIntReturningInt );

    // "new" style C code - don't need "&" for function type.
    functionTakingFunctionPointerNewStyle( NULL );
    functionTakingFunctionPointerNewStyle( FuncionTakingIntReturningInt );

    // blocks
    functionTakingBlock( NULL );
    functionTakingBlock( ^(int i){ return i + 1; } );

}

Output:

old style pf = 0x0

old style pf = 0x1004aa980
pf(12) = 13

new style pf = 0x0

new style pf = 0x1004aa980
pf(12) = 13

block pf = 0x0

block pf = 0x1004ab0f0
pf(12) = 13