» open closed principle » Managed Chaos by Naresh Jain

OO Design Principles

Thursday, June 7th, 2012

Following Object Oriented Design Principles have really helped me designing my code:

Single Responsibility Principle
Open Closed Principle
Liskov Substitution Principle or Design by Contract
Interface Segregation Principle
Dependency InversionPrinciple
Once and Only Once
DRY – Don’t Repeat yourself
Tell Don’t Ask
The Law of Demeter
Triangulate – When you are not sure what the correct abstraction should be, instead of pulling out an abstraction upfront, you get the second case to work by duplicating and modifying a small piece of code. Once you have both the solutions working, find the “generic” form and create an abstraction

Along with these principles, I’ve also learned a lot from the 17 rules explained in the Art of Unix Programming book:

Rule of Modularity: Write simple parts connected by clean interfaces
Rule of Clarity: Clarity is better than cleverness.
Rule of Composition: Design programs to be connected to other programs.
Rule of Separation: Separate policy from mechanism; separate interfaces from engines
Rule of Simplicity: Design for simplicity; add complexity only where you must
Rule of Parsimony: Write a big program only when it is clear by demonstration that nothing else will do
Rule of Transparency: Design for visibility to make inspection and debugging easier
Rule of Robustness: Robustness is the child of transparency and simplicity
Rule of Representation: Fold knowledge into data so program logic can be stupid and robust
Rule of Least Surprise: In interface design, always do the least surprising thing
Rule of Silence: When a program has nothing surprising to say, it should say nothing
Rule of Repair: When you must fail, fail noisily and as soon as possible
Rule of Economy: Programmer time is expensive; conserve it in preference to machine time
Rule of Generation: Avoid hand-hacking; write programs to write programs when you can
Rule of Optimization: Prototype before polishing. Get it working before you optimize it
Rule of Diversity: Distrust all claims for “one true way”
Rule of Extensibility: Design for the future, because it will be here sooner than you think

Posted in Agile, Code Smells, Design, Programming | 1 Comment »

Refactoring Teaser 1: Take 1

Tuesday, July 14th, 2009

Last week I posted a small code snippet for refactoring under the heading Refactoring Teaser.

In this post I’ll try to show step by step how I would try to refactor this mud ball.

First and foremost cleaned up the tests to communicate the intent. Also notice I’ve changed the test class name to ContentTest instead of StringUtilTest, which means anything and everything.

public class ContentTest {
    private Content helloWorldJava = new Content("Hello World Java");
    private Content helloWorld = new Content("Hello World!");
 
    @Test
    public void ignoreContentSmallerThan3Words() {
        assertEquals("", helloWorld.toString());
    }
 
    @Test
    public void buildOneTwoAndThreeWordPhrasesFromContent() {
        assertEquals("'Hello', 'World', 'Java', 'Hello World', 'World Java', 'Hello World Java'", helloWorldJava.toPhrases(6));
    }
 
    @Test
    public void numberOfOutputPhrasesAreConfigurable() {
        assertEquals("'Hello'", helloWorldJava.toPhrases(1));
        assertEquals("'Hello', 'World', 'Java', 'Hello World'", helloWorldJava.toPhrases(4));
    }
 
    @Test
    public void returnsAllPhrasesUptoTheNumberSpecified() {
        assertEquals("'Hello', 'World', 'Java', 'Hello World', 'World Java', 'Hello World Java'", helloWorldJava.toPhrases(10));
    }
}

Next, I created a class called Content, instead of StringUtil. Content is a first-class domain object. Also notice, no more side-effect intense statics.

public class Content {
    private static final String BLANK_OUTPUT = "";
    private static final String SPACE = " ";
    private static final String DELIMITER = "', '";
    private static final String SINGLE_QUOTE = "'";
    private static final int MIN_NO_WORDS = 2;
    private static final Pattern ON_WHITESPACES = Pattern.compile("\\p{Z}|\\p{P}");
    private List phrases = new ArrayList();
 
    public Content(final String content) {
        String[] tokens = ON_WHITESPACES.split(content);
        if (tokens.length &gt; MIN_NO_WORDS) {
            buildAllPhrasesUptoThreeWordsFrom(tokens);
        }
    }
 
    @Override
    public String toString() {
        return toPhrases(Integer.MAX_VALUE);
    }
 
    public String toPhrases(final int userRequestedSize) {
        if (phrases.isEmpty()) {
            return BLANK_OUTPUT;
        }
        List requiredPhrases = phrases.subList(0, numberOfPhrasesRequired(userRequestedSize));
        return withInQuotes(join(requiredPhrases, DELIMITER));
    }
 
    private String withInQuotes(final String phrases) {
        return SINGLE_QUOTE + phrases + SINGLE_QUOTE;
    }
 
    private int numberOfPhrasesRequired(final int userRequestedSize) {
        return userRequestedSize &gt; phrases.size() ? phrases.size() : userRequestedSize;
    }
 
    private void buildAllPhrasesUptoThreeWordsFrom(final String[] words) {
        buildSingleWordPhrases(words);
        buildDoubleWordPhrases(words);
        buildTripleWordPhrases(words);
    }
 
    private void buildSingleWordPhrases(final String[] words) {
        for (int i = 0; i &lt; words.length; ++i) {
            phrases.add(words[i]);
        }
    }
 
    private void buildDoubleWordPhrases(final String[] words) {
        for (int i = 0; i &lt; words.length - 1; ++i) {
            phrases.add(words[i] + SPACE + words[i + 1]);
        }
    }
 
    private void buildTripleWordPhrases(final String[] words) {
        for (int i = 0; i &lt; words.length - 2; ++i) {
            phrases.add(words[i] + SPACE + words[i + 1] + SPACE + words[i + 2]);
        }
    }
}

public class Content { private static final String BLANK_OUTPUT = ""; private static final String SPACE = " "; private static final String DELIMITER = "', '"; private static final String SINGLE_QUOTE = "'"; private static final int MIN_NO_WORDS = 2; private static final Pattern ON_WHITESPACES = Pattern.compile("\\p{Z}|\\p{P}"); private List phrases = new ArrayList(); public Content(final String content) { String[] tokens = ON_WHITESPACES.split(content); if (tokens.length > MIN_NO_WORDS) { buildAllPhrasesUptoThreeWordsFrom(tokens); } } @Override public String toString() { return toPhrases(Integer.MAX_VALUE); } public String toPhrases(final int userRequestedSize) { if (phrases.isEmpty()) { return BLANK_OUTPUT; } List requiredPhrases = phrases.subList(0, numberOfPhrasesRequired(userRequestedSize)); return withInQuotes(join(requiredPhrases, DELIMITER)); } private String withInQuotes(final String phrases) { return SINGLE_QUOTE + phrases + SINGLE_QUOTE; } private int numberOfPhrasesRequired(final int userRequestedSize) { return userRequestedSize > phrases.size() ? phrases.size() : userRequestedSize; } private void buildAllPhrasesUptoThreeWordsFrom(final String[] words) { buildSingleWordPhrases(words); buildDoubleWordPhrases(words); buildTripleWordPhrases(words); } private void buildSingleWordPhrases(final String[] words) { for (int i = 0; i < words.length; ++i) { phrases.add(words[i]); } } private void buildDoubleWordPhrases(final String[] words) { for (int i = 0; i < words.length - 1; ++i) { phrases.add(words[i] + SPACE + words[i + 1]); } } private void buildTripleWordPhrases(final String[] words) { for (int i = 0; i < words.length - 2; ++i) { phrases.add(words[i] + SPACE + words[i + 1] + SPACE + words[i + 2]); } } }

This was a big step forward, but not good enough. Next I focused on the following code:

    private void buildAllPhrasesUptoThreeWordsFrom(final String[] words) {
        buildSingleWordPhrases(words);
        buildDoubleWordPhrases(words);
        buildTripleWordPhrases(words);
    }
 
    private void buildSingleWordPhrases(final String[] words) {
        for (int i = 0; i &lt; words.length; ++i) {
            phrases.add(words[i]);
        }
    }
 
    private void buildDoubleWordPhrases(final String[] words) {
        for (int i = 0; i &lt; words.length - 1; ++i) {
            phrases.add(words[i] + SPACE + words[i + 1]);
        }
    }
 
    private void buildTripleWordPhrases(final String[] words) {
        for (int i = 0; i &lt; words.length - 2; ++i) {
            phrases.add(words[i] + SPACE + words[i + 1] + SPACE + words[i + 2]);
        }
    }

The above code violates the Open-Closed Principle (pdf). It also smells of duplication. Created a somewhat generic method to kill the duplication.

    private void buildAllPhrasesUptoThreeWordsFrom(final String[] fromWords) {
        buildPhrasesOf(ONE_WORD, fromWords);
        buildPhrasesOf(TWO_WORDS, fromWords);
        buildPhrasesOf(THREE_WORDS, fromWords);
    }
 
    private void buildPhrasesOf(final int phraseLength, final String[] tokens) {
        for (int i = 0; i &lt;= tokens.length - phraseLength; ++i) {
            String phrase = phraseAt(i, tokens, phraseLength);
            phrases.add(phrase);
        }
    }
 
    private String phraseAt(final int currentIndex, final String[] tokens, final int phraseLength) {
        StringBuilder phrase = new StringBuilder(tokens[currentIndex]);
        for (int i = 1; i &lt; phraseLength; i++) {
            phrase.append(SPACE + tokens[currentIndex + i]);
        }
        return phrase.toString();
    }

Now I had a feeling that my Content class was doing too much and also suffered from the primitive obsession code smell. Looked like a concept/abstraction (class) was dying to be called out. So created a Words class as an inner class.

    private class Words {
        private String[] tokens;
        private static final String SPACE = " ";
 
        Words(final String content) {
            tokens = ON_WHITESPACES.split(content);
        }
 
        boolean has(final int minNoWords) {
            return tokens.length &gt; minNoWords;
        }
 
        List phrasesOf(final int length) {
            List phrases = new ArrayList();
            for (int i = 0; i &lt;= tokens.length - length; ++i) {
                String phrase = phraseAt(i, length);
                phrases.add(phrase);
            }
            return phrases;
        }
 
        private String phraseAt(final int index, final int length) {
            StringBuilder phrase = new StringBuilder(tokens[index]);
            for (int i = 1; i &lt; length; i++) {
                phrase.append(SPACE + tokens[index + i]);
            }
            return phrase.toString();
        }
    }

In the constructor of the Content class we instantiate a Words class as follows:

    public Content(final String content) {
        Words words = new Words(content);
        if (words.has(MIN_NO_WORDS)) {
            phrases.addAll(words.phrasesOf(ONE_WORD));
            phrases.addAll(words.phrasesOf(TWO_WORDS));
            phrases.addAll(words.phrasesOf(THREE_WORDS));
        }
    }

Even though this code communicates well, there is duplication and noise that can be removed without compromising on the communication.

     phrases.addAll(words.phrasesOf(ONE_WORD, TWO_WORDS, THREE_WORDS));

There are few more version after this, but I think this should give you an idea about the direction I’m heading.

Posted in Agile, Design, Programming, Testing | No Comments »