The random utterances of David Arno

Sometime ago, I wrote what turned out to be a highly controversial post in which I argued that private members should be avoided as they caused more problems than they solved. Two years on, I have come to realise that I was wrong and that problems with private members are simply symptoms of bad written code, rather than problems in their own right.

At the time, I noticed that most of the people agreeing with my anti-private stance were – like me – Flex developers. If you want to see an example of badly written OO code, in which the use of the private keyword causes endless problems, I’d recommend taking a look at some of the Flex framework source. A lack of interfaces, gigantic classes and massive use of inheritance make it a tough framework to work with. Having important functionality hidden away behind private methods makes a bad situation far worse. At the time, the solution to me seemed simple: ban the use of private, for it was clearly the cause of many of the problems with using the Flex framework. Or so that was the way I saw it.

Time moves on and – like any half-decent developer – I have continued to expand my understanding of, and skills with, software development. Through conferences like QCon and try {harder} and with help from folk who, instead of just saying “you are wrong” sought to teach me why, I have come to understand that private members are a symptom of bad OO design, not the cause of it. Some of these people have taught me much and deserve a name check: I’m thinking of you Till Schneidereit, Stray (Lindsey Fallow) and Shaun Smith.

To understand why I had a problem with the private keyword, consider the following real-world example. In an application I was working on, we used the Flex 3 component NumericStepper. This component is a text field with up and down buttons. A numeric value can be entered, or the up/down buttons can be used to select the desired value. In one case, we used the component to set a value with one decimal place. Our QA department raised an issue against it as it would advance up through numbers thus: “1.8″, “1.9″, “2″. The significant decimal place was lost on a whole number and they wanted it fixed so it would advance as “1.8″, “1.9″, “2.0″. To fix this, I delved into the source of the component to see what was going on. The method responsible for setting the value was:

    private function setValue(value:Number,
                              sendEvent:Boolean = true,
                              trigger:Event = null):void
    {
 
        var v:Number = checkValidValue(value);
        if (v == lastValue)
            return;
 
        lastValue = _value = v;
        inputField.text = v.toString();
 
        if (sendEvent)
        {
            var event:NumericStepperEvent =
                new NumericStepperEvent(NumericStepperEvent.CHANGE);
            event.value = _value;
            event.triggerEvent = trigger;
 
            dispatchEvent(event);
        }
 
        dispatchEvent(new FlexEvent(FlexEvent.VALUE_COMMIT));
    }

The key line was inputField.text = v.toString();. All I had to do was change this behaviour. But there was a problem: the method was private. I couldn’t change it’s behaviour. We ended up creating a special case TextInput class, overriding its text setter, which formatted the text as we needed it, and then forced the NumericStepper instance to use our new class through a series of hacks and bodges to the code. The code was unintuitive and required lots of “this is why we are doing this weird shit” comments to explain why we were doing it the way we were. All because a vital method had been marked private. This is just one of many cases where the use of the private keyword in the Flex framework makes life hard for users of the framework. We actually seriously considered doing a global replace of all occurrences of private with protected and rebuilding our own version of the framework. It is unsurprising therefore that I concluded the the use of private was a bad thing and ought to stop.

At this point, it’s time to introduce the SOLID principle. A good OO design will obey the following principles:

• Single responsibility principle: an object should have only a single responsibility.
• Open/closed principle – classes should be open for extension, but closed for modification.
• Liskov substitution principle – objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
• Interface segregation principle – many client specific interfaces are better than one general purpose interface.
• Dependency inversion principle – one should depend upon Abstractions. Do not depend upon concretions.

An examination of the Flex framework quickly reveals that its formed from a set of gigantic classes, many of which are thousands of lines long, each with many responsibilities. They are often closed to easy extension, such as shown above in the NumericStepper class. Classes such as the Menu class break the Liskov substitution principle by extending the List class and then crudely hiding list functionality such as scrollbars. There are very few interfaces in Flex, much less many specific interfaces, so the interface segregation principle is ignored too. Finally, it’s heavy use of locator patterns and singletons mean it doesn’t work well with injected abstractions. It is hardly surprising that a framework that breaks all the SOLID principles also suffers from misuse of the private keyword. When looked at this way, I started to realise that maybe the private keyword wasn’t the problem.

In the last year or so, I’ve adopted a very simple rule for the code I write and that those I work with have been bullied into adopting too: a class should aim to be less than 100 lines and must not be more than 200 lines. This simple rule has a profound effect upon code. Classes become simpler and more focused. Rather than having to extend existing classes, simply implementing another version of an interface becomes the simpler solution. Those interfaces need to be numerous and focused on a few methods to keep implementing classes small and dependency injection tends to get used without even thinking about it. In short, the SOLID principle almost falls out of small classes. It stops being a principle that one must struggle to achieve and becomes a by-product instead.

Small classes had another effect too: protected methods become unnecessary. As classes tend to be replaced with alternate interface implementations, rather than being extended, there are no need for protected methods. Testing the public methods of a small class tests its internals too and so there becomes no need to expose those internals for testing. In short, those internals become implementation details that can be marked private. In so doing, the open/closed principle neatly comes into effect.

In summary, I’m still glad I wrote my anti-private article two years ago: it forced me to put into words those frustrations I was suffering back then. Two years on I’m older and wiser and now understand I was wrong in what I wrote. Private members are a good idea, but only in well written code. So I now write my code better than I did before and I have started using the private keyword once more.