mercredi 8 juin 2016

Polymorphic directives in AngularJs


This is a short write up on how to perform ReplaceConditionalWithPolymorphism for directives in angular

Recently I had to add some functionality to a menu-bar that handles navigation between different screens. It is implemented as a directive. The problem was the same directive was instantiated by several controllers, and from some controllers it needs an instance of something, lets call it Book, and from one other controller it needs an instance something else that we’ll call Page.

I’m going to use the syntax of javascript classes to explain the problem, as it is rather clear and not specific to a framework. At the end we’ll look at the angular version.

class MenuBar {
     constructor(book, page) {
          // either of book or page are undefined
     }

     goToView(viewItem) {
    if (onBookView()) {   // i.e. page == undefined
        handleRedirectionFromBookViewTo(viewItem)
    } else // on page view
        handleRedirectionFromPageViewTo(viewItem)
    }
}

}

It is awkward and subject to error to allow either book or page to be null/undefined as it is difficult for another developer to know what is a valid way of instantiating it. Can both be undefined? If I define page, to I have to provide book? As far as testing goes, we have little confidence that it is instantiated with the right arguments in every view, so we're kind of forced to test all functionality in every view to be sure we don’t get a runtime failure. A combinatorial problem.

In addition much of the cyclomatic complexity of this class is unnecessary. Basically one set of branches are used when we have a book and a completely different one when we have a page. This is a clear cut for polymorphism :

class MenuBarForBook {
     constructor(book) {...}
    
     goToView(viewItem) {
    handleRedirectionFromBookViewTo(viewItem)
     }

}

class  MenuBarForPage {
     constructor(page) {...}

     goToView(viewItem) {
     handleRedirectionFromPageViewTo(viewItem)
     }
}

With this design it is unlikely that a constructor won't be called with the right arguments. The combinatorial problem is solved and we've lowered the testing burden.

So how can we do this in angular. Easy; create two different directives that expose the same functions and use the same template.

angular.module('menuBar', [])
    .directive('menuBarBook', [
    function () {
        return {
            templateUrl'src/menu/MenuBar.html',
            scope: {
                book‘=‘    // constructor argument
            },
            linkfunction ($scope) {
                $scope.goToView function () {    // function goToView()
                    handleRedirectionFromBookViewTo(viewItem)
                }
            }
        }
    }])
    .directive('menuBarPage', [
    function () {
        return {
            templateUrl'src/menu/MenuBar.html',
            scope: {
                page'=',
            },
            linkfunction($scope) {
                $scope.goToView function () {
                    handleRedirectionFromPageViewTo(viewItem)
                }
            }
        }
    }])

Conclusion

Polymorphism is a powerful tool both for making the code more usable to other developers by making it more explicit how to use a piece of code. Also, removing if-statements not only makes the code simpler but also makes the tests simpler. I just discovered this could be done with directives without too much overhead. It is worth naming directive inheritance here. However to my understanding it solves a different problem, namely sharing code.

jeudi 2 juin 2016

It's not a configuration issue. It’s a design issue

Consider this piece of code

url applicationConfig.publicationUrl relativeResourcePath
it contains a source of error, namely data.relativeResourcePath is of the form files/document.pdf and publicationUrl is 
of the form http://someserver/. So when someone changes the the applicationConfig to say http://otherServer:81 the download is not going to work anymore as we dropped the trailing slash. So url would be http://otherServer:81files/document.pdf which obviously won’t work

Ok so this is a configuration issue. What a pity, can’t do much about that. Even if we do TDD, testing configuration effectively isn’t easy. 

But wait! This code suffers from a Code Smell called Primitive Obsession. A better design would be the following:

url applicationConfig.publicationUrlFor(relativeResourcePath)
Poka Yoke

Now it’s easy to add the extra slash in case it is missing within the publicationUrlFor method and this type of configuration issue is gone forever throughout the application. Besides publicationUrlFor() is testable.

This is a simple example to highlight the idea that we can design away many of the bugs that we live with in our projects. By designing away sources of errors we make our code more usable, it can only be used in the right way. In Lean this is called Poka-Yoke. Every situation needs a different design to eliminate the possibility of misuse, but the general idea is the same - Design away sources of bugs.


For inspiration have a look at this other example of how to design away bugs due to temporal coupling: section Eliminate Exceptions in this post.