Archive for the 'Code Review' Category

Some useful Tips to keep in mind

Sunday, February 28th, 2010

Throughout my career I have always been drawn to books which provide a practical way of thinking about software. Books of this nature have an emphasis on the fundamental principles which apply to all software engineering disciplines; and form much of the basis of the Agile methodologies many of us have come to appreciate.

From time to time I find myself going back to the seminal text The Pragmatic Programmer as it provides a great source of what should be kept in mind from day to day. I thought I would share some tips from the book with my readers that I find of particular value, and would be happy to explain them at length; in the context of real world software challenges.

Care About Your Craft
Why spend your life developing software unless you care about doing it well?

Provide Options, Don’t Make Lame Excuses
Instead of excuses, provide options. Don’t say it can’t be done; explain what can be done.

Critically Analyze What You Read and Hear
Don’t be swayed by vendors, media hype, or dogma. Analyze information in terms of you and your project.

Design with Contracts
Use contracts to document and verify that code does no more and no less than it claims to do.

Refactor Early, Refactor Often
Just as you might weed and rearrange a garden, rewrite, rework, and re-architect code when it needs it. Fix the root of the problem.

Costly Tools Don’t Produce Better Designs
Beware of vendor hype, industry dogma, and the aura of the price tag. Judge tools on their merits.

Start When You’re Ready
You’ve been building experience all your life. Don’t ignore niggling doubts.

Don’t Be a Slave to Formal Methods
Don’t blindly adopt any technique without putting it into the context of your development practices and capabilities.

It’s Both What You Say and the Way You Say It
There’s no point in having great ideas if you don’t communicate them effectively.

You Can’t Write Perfect Software
Software can’t be perfect. Protect your code and users from the inevitable errors.

Build Documentation In, Don’t Bolt It On
Documentation created separately from code is less likely to be correct and up to date.

Put Abstractions in Code, Details in Metadata
Program for the general case, and put the specifics outside the compiled code base.

Work with a User to Think Like a User
It’s the best way to gain insight into how the system will really be used.

Program Close to the Problem Domain
Design and code in your user’s language.

Use a Project Glossary
Create and maintain a single source of all the specific terms and vocabulary for a project.

Be a Catalyst for Change
You can’t force change on people. Instead, show them how the future might be and help them participate in creating it.

DRY - Don’t Repeat Yourself
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Eliminate Effects Between Unrelated Things
Design components that are self-contained, independent, and have a single, well-defined purpose.

Iterate the Schedule with the Code
Use experience you gain as you implement to refine the project time scales.

Use the Power of Command Shells
Use the shell when graphical user interfaces don’t cut it.

Don’t Panic When Debugging
Take a deep breath and THINK! about what could be causing the bug.

Don’t Assume It - Prove It
Prove your assumptions in the actual environment—with real data and boundary conditions.

Write Code That Writes Code
Code generators increase your productivity and help avoid duplication.

Test Your Software, or Your Users Will
Test ruthlessly. Don’t make your users find bugs for you.

Don’t Gather Requirements—Dig for Them
Requirements rarely lie on the surface. They’re buried deep beneath layers of assumptions, misconceptions, and politics.

Abstractions Live Longer than Details
Invest in the abstraction, not the implementation. Abstractions can survive the barrage of changes from different implementations and new technologies.

Don’t Think Outside the Box—Find the Box
When faced with an impossible problem, identify the real constraints. Ask yourself: “Does it have to be done this way? Does it have to be done at all?”;

Some Things Are Better Done than Described
Don’t fall into the specification spiral—at some point you need to start coding.

Don’t Use Manual Procedures
A shell script or batch file will execute the same instructions, in the same order, time after time.

Test State Coverage, Not Code Coverage
Identify and test significant program states. Just testing lines of code isn’t enough.

Gently Exceed Your Users’ Expectations
Come to understand your users’ expectations, then deliver just that little bit more.

Don’t Live with Broken Windows
Fix bad designs, wrong decisions, and poor code when you see them.

Remember the Big Picture
Don’t get so engrossed in the details that you forget to check what’s happening around you.

Make It Easy to Reuse
If it’s easy to reuse, people will. Create an environment that supports reuse.

There Are No Final Decisions
No decision is cast in stone. Instead, consider each as being written in the sand at the beach, and plan for change.

Estimate to Avoid Surprises
Estimate before you start. You’ll spot potential problems up front.

Use a Single Editor Well
The editor should be an extension of your hand; make sure your editor is configurable, extensible, and programmable.

Fix the Problem, Not the Blame
It doesn’t really matter whether the bug is your fault or someone else’s—it is still your problem, and it still needs to be fixed.

“select” Isn’t Broken
It is rare to find a bug in the OS or the compiler, or even a third-party product or library. The bug is most likely in the application.

Learn a Text Manipulation Language
You spend a large part of each day working with text. Why not have the computer do some of it for you?

Use Exceptions for Exceptional Problems
Exceptions can suffer from all the readability and maintainability problems of classic spaghetti code. Reserve exceptions for exceptional things.

Minimize Coupling Between Modules
Avoid coupling by writing “shy” code and applying the Law of Demeter.

Design Using Services
Design in terms of services: independent, concurrent objects behind well-defined, consistent interfaces.

Don’t Program by Coincidence
Rely only on reliable things. Beware of accidental complexity, and don’t confuse a happy coincidence with a purposeful plan.

Organize Teams Around Functionality
Don’t separate designers from coders, testers from data modelers. Build teams the way you build code.

Test Early. Test Often. Test Automatically.
Tests that run with every build are much more effective than test plans that sit on a shelf.

Find Bugs Once
Once a human tester finds a bug, it should be the last time a human tester finds that bug. Automatic tests should check for it from then on.

Sign Your Work
Craftsmen of an earlier age were proud to sign their work. You should be, too.

I suggest keeping these tips, or a subset of them, as well as some of your own, somewhere visible; even if some appear rather obvious, as it will serve as a good general reminder of the things you should always keep in mind.

Design Considerations: Naming Conventions

Friday, July 31st, 2009

Intuitive naming conventions are perhaps one of the most important factors in providing a scalable software system. They are essential to ensuring an Object Oriented System can easily be understood, and thus modified by all members of a team regardless of their tenure within the organization or individual experience level.

When classes, interfaces, methods, properties, identifiers, events and the like fail to follow logical, consistent and intuitive naming conventions the resulting software becomes significantly more complex to understand, follow and maintain. As such this makes changes much more challenging than they would have been had better naming been considered originally. Of equal concern is the inevitability that poor naming will lead to redundant code being scattered throughout a project as when the intent of code is not clearly conveyed with as little thought as possible developers tend to re-implement existing functionality when the needed API cannot easily be located or identified.

Code is typically read many, many more times than it is written. With this in mind it is important to understand that the goal of good naming is to be as clear and concise as possible so that a reader of the code can easily determine the codes intent and purpose; just by reading it.

Teams should collectively define a set of standard naming conventions which align well with the typical conventions found in their language of choice. In doing so this will help to avoid arbitrary naming conventions which often result in code that is significantly harder to determine intent, and thus maintain. Of equal importance is the need for various teams from within the same engineering department to standardize on domain specific terms which align with the non-technical terms used by business stakeholders. Together this will help to develop a shared lexicon between business owners and engineers, and allow for simplified analysis of requirements etc.

Ideally, code should follow the PIE Principle (Program, Intently and expressively) - that is, code should clearly convey purpose and intent. In doing so the ability to maintain a software application over time becomes significantly easier and limits the possibility of introducing potential risk to project deliverables.

In short, conventions are very important regardless of a teams size; beit a large collaborative team environment, or a single developer who only deals with his own code. Consistency and conventions are a key aspect to ensuring code quality.

Cairngorm Abstractions: Business Delegates

Thursday, May 7th, 2009

In Part 1 of Cairngorm Abstractions I discussed the common patterns which can be utilized in a design to simplify the implementation of concrete Cairngorm Commands and Responders. Applying such patterns can be leveraged to help facilitate code reuse and provide a maintainable, scalable architecture, as, in doing so the design will ultimately ensure reuse as well as remove redundancy.

In this post I will describe the same benefits which can be gained by defining common abstractions of Business Delegates.

Business Delegate Abstractions
A Business Delegate should provide an interface against the service to which it references. This can be viewed as a one-to-one relationship whereas the operations and signatures defined by a Service, beit an HTTPService, WebService, RemoteObject, DataService etc. would dictate the Business Delegate’s API.

However, a rather common mistake I often find is that many times Business Delegates are defined in the context of the use case which invokes them, rather than the service from which they provide an interface against.

Correcting this is quite simple: refactor the current implementation to follow the one-to-one relationship model between a Service and Business Delegate.

So for instance, if your applications service layer specifies a “UserService”, your design should essentially have only one Business Delegate API for that Service. All of the operations provided by the “UserService” would be defined by an “IUserServiceDelegate” interface which would enforce the contract between the “UserService” and concrete Delegate implementations, regardless of their underlying service mechanism.

In this manner clients (delegate instances) can be defined as the abstraction (IUserServiceDelegate) and obtain references to concrete Business Delegate instances via a Delegate Factory, and as such remain completely transparent of their underlying service implementation.

This could be implemented as follows:

var delegate:IUserServiceDelegate;
delegate = DelegateFactory.createUserServiceDelegate( responder );
// invoke delegate …

Abstract Delegates
Perhaps the most common design improvement which can be made to improve the implementation and maintainability of Business Delegates is to define proper abstractions which provide an implementation which is common amongst all Business Delegates. Additionally, in doing so you will remove a significant amount of redundancy from your design.

For example, if you compare any two Business Delegates and find they have practically the exact same implementation, that is an obvious sign that a common abstraction should be defined.

Consider the following Business Delegate implementation:

public class SomeDelegate
{   
  private var _service:RemoteObject;
  private var _responder:IResponder;
   
  public function SomeDelegate(responder:IResponder)
  {
      _service = ServiceLocator.getInstance().
                 getRemoteObject( Services.LOGIN_SERIVCE );
      _responder = responder;
  }
   
  public function methodA(arg1:String, arg2:int) : void
  {
      var call:AsyncToken = _service.methodA( arg1, arg2);
      call.addResponder( _responder );
  }

  public function methodB(arg:Boolean) : void
  {
      var call:AsyncToken = _service.methodB( arg );
      call.addResponder( _responder );
  }

  public function methodC() : void
  {
      var call:AsyncToken = _service.methodC();
      call.addResponder( _responder );
  }
  …
}

The above example may look familiar, and when given just a bit of thought as to it’s design it becomes apparent that there is quite a bit of redundancy as every method essentially contains the same implementation code. That is, an AsyncToken is created, referencing the operation to invoke against the service, and a reference to the responder is added to the token.

The overall design would benefit much more by refactoring the commonality implemented across all Business Delegate methods to an abstraction, which in it’s simplest form could be defined as follows:

public class AbstractRemoteObjectDelegate
{   
  protected var service:RemoteObject;
  protected var responder:IResponder;

  public function AbstractRemoteObjectDelegate(serviceId:String,
                                            responder:IResponder)
  {
      this.service = ServiceLocator.
                     getInstance().getRemoteObject( serviceId );
      this.responder = responder;
  }

  protected function invoke(methodName:String, …args) : void
  {
      var operation:Operation = service[ methodName ];
      operation.arguments = args;
           
      var call:AsyncToken = operation.send();
      call.addResponder( responder );
  }
}

By defining a basic abstraction, the original implementation could then be refactored to the following:

public class SomeDelegate extends AbstractRemoteObjectDelegate
{   
  public function SomeDelegate(responder:IResponder)
  {
      super( Services.LOGIN_SERIVCE, responder );
  }
   
  public function methodA(arg1:String, arg2:int) : void
  {
      invoke( "methodA", arg1, arg2 );
  }

  public function methodB(arg:Boolean) : void
  {
      invoke( "methodB", arg );
  }

  public function methodC() : void
  {
      invoke( "methodC" );
  }
}

The same basic abstractions could easily be defined for HTTPService, WebService and DataService specific Business Delegates (in fact I have a library of Cairngorm extensions which provides them; planning on releasing these soon). Pulling up common implementation code to higher level abstract types also simplifies writing tests against concrete Business Delegates as the abstraction itself would need only to be tested once.

There are many more Business Delegate abstractions I would recommend in addition to what I have outlined here, in particular configuring Delegate Factories via an IoC Container such as SAS, however I would first suggest taking a good look at your current design before adding additional layers of abstraction, and the most appropriate place to start would be to define abstractions which encapsulate commonality, promote reuse and remove redundancy.

Cairngorm Abstractions: Commands and Responders

Tuesday, April 21st, 2009

It is quite common to find a significant amount of code redundancy in Flex applications built on Cairngorm. This is by no means a fault of the framework itself, actually quite the contrary as Cairngorm is designed with simplicity in mind; opting to appropriately take a less-is-more approach in favor of providing a more prescriptive framework which only defines the implementation classes necessary to facilitate the “plumbing” behind the framework. Everything else is really just an interface.

With this amount of flexibility comes additional responsibility in that developers must decide what the most appropriate design is based on their applications specific context. Moreover, as with any design there is never a truly one size fits all approach which can be applied to any problem domain; there are really only common patterns and conventions which can be applied across domains and applications. This IMHO is what had allowed the framework to be a success and it is important to understand that this simplicity also requires developers to give their designs the same attention one would to any Object Oriented design.

However over the years I have found a significant amount of redundancy found in Flex applications built on Cairngorm. This appears to be (more often than not) the result of developers implementing Cairngorm examples verbatim in real world applications, and in doing so failing to define proper abstractions for commonly associated concerns and related responsibilities. The most common example of this is the typical implementation of Commands, Responders BusinessDelegates and PresentationModel implementations.

For some of you this may all seem quite obvious, and for others hopefully this series will provide some insight as to how one can reduce code redundancy across your Cairngorm applications by implementing abstractions for common implementations.

This topic will be a multi-part series in which I will provide some suggestions surrounding the common patterns of abstractions which can be implemented in an application built on Cairngorm, with this first installment based on common abstractions of Cairngorm Commands and Responders. Other areas in future posts will cover Business Delegate and Presentation Model abstractions. So let’s get started…

Command Abstractions
First let’s begin by looking at what is arguably the simplest abstraction one could define in a Cairngorm application to simplify code and eliminate areas of redundancy - Command abstractions. This example assumes the concern of mx.rpc.IResponder implementations is abstracted to a separate object. For more on this subject see my post regarding IResponder and Cairngorm.

A traditional Cairngorm Command is typically implemented as something to the extent of the following:

import com.adobe.cairngorm.commands.ICommand;
import com.adobe.cairngorm.control.CairngormEvent;

public class Command implements ICommand
{
  public function execute(event:CairngormEvent):void
  {
    var evt:SomeEvent = event as SomeEvent;
           
    // ModelLocator look-up and common references
    ModelLocator.getInstance()
}
}

The problem with the above Command implementation is that it results in numerous look-ups on the ModelLocator Singleton instance in every execute implementation which needs to reference the ModelLocator.

A simpler design would be to define an abstraction for all commands which contains this reference. as in the following:

import com.adobe.cairngorm.commands.ICommand;
import com.adobe.cairngorm.control.CairngormEvent;
   
/**
 *
 * Defines an abstraction of common references from
 * which concrete ICommand implementations can
 * inherit
 *
 */

internal class AbstractCommand implements ICommand
{
  // define common reference to ModelLocator
  // implementation
  protected static var modelLocator:ModelLocator
                       = ModelLocator.getInstance();

  // Force concrete command implementations to
  // override execute
  public function execute(event:CairngormEvent) : void
  {
    throw new Error( "Abstract operation…" );
  }
}

As in any OO system there are many benefits to defining abstractions and a good design certainly reflects this. For example, just by defining a very basic abstraction for all Commands we have now eliminated the number of look-ups on the ModelLocator for every Command in the application as well as redundant imports. By defining an abstraction for common references your code will become easier to read and maintain as the number of lines of code will certainly become reduced.

Commands are by far the easiest to create an abstraction for as most commands will typically reference the ModelLocator, and if so they could do so simply by extending an AbstractCommand, if not they would implement ICommand as they traditionally would.

So the first example could now be refactored to the following:

import com.adobe.cairngorm.control.CairngormEvent;

public final class Command extends AbstractCommand
{
  override public function execute(event:CairngormEvent):void
  {
    var evt:SomeEvent = event as SomeEvent;
    // modelLocator…
  }
}

You could take these abstractions a step further and define additional abstractions for related behavior and contexts, all of which would also extend the AbstractCommand if a reference to the applications ModelLocator is needed.

Responder Abstractions
Now let’s take a look at an abstraction which is much more interesting - Responder abstractions. In this example we will focus on the most common Responder implementation; mx.rpc.IResponder, however the same could easily apply for an LCDS Responder implementation of a DataService.

A separate RPC responder could be defined as an abstraction for HTTPServices, WebServices and RemoteObjects as each request against any of these services results in a response of either result or fault, hence the IResponder interface’s contract.

For example, consider a typical Responder implementation which could be defined as follows:

import mx.rpc.IResponder;
import mx.rpc.events.FaultEvent;
import mx.rpc.events.ResultEvent;
   
public class SomeResponder implements IResponder
{      
  public function result(data:Object) : void
  {
    // redundant cast operation
    var result:ResultEvent = data as ResultEvent;
           
    // Redundant ModelLocator lookup and references…
    // ModelLocator.getInstance()…
  }

  public function fault(info:Object) : void
  {
    // Redundant cast operation
    // Doesn’t provide a centralized place for
    // global service exception handling
    var fault:FaultEvent = info as FaultEvent;
           
    // Redundant ModelLocator lookup and references…
    // ModelLocator.getInstance()…
  }
}

By defining a Responder abstraction each concrete Responder implementation would result in significantly less code as the redundant cast operations could be abstracted, and, as with Command Abstractions, a convenience reference to the application specific ModelLocator could also be defined. Moreover, a default service fault implementation could be defined from which each service fault could be handled uniformly across the application.

Thus we could define an abstracttion for RPC Responders as follows:

import mx.rpc.IResponder;
import mx.rpc.events.FaultEvent;
import mx.rpc.events.ResultEvent;
   
/**
 *
 * Defines an abstraction of common references and
 * functionality from which concrete IResponder
 * implementations can inherit
 *
 */

internal class AbstractRPCResponder implements IResponder
{   
  protected static var modelLocator:ModelLocator
                       = ModelLocator.getInstance();
           
  // Provides a default implementation of
  // mx.rpc.IResponder.result(); which
  // handles casting to a ResultEvent
  public function result(data:Object):void
  {
    var result:ResultEvent = ResultEvent( data );
    resultResponse( result );
  }
       
  // provide default implementation of
  // mx.rpc.IResponder.fault(); which
  // handles casting to a FaultEvent
  public function fault(info:Object) : void
  {
    var fault:FaultEvent = FaultEvent( info );
    faultResponse( fault );
  }
       
  // Force concrete implementation to override
  // resultResponse
  public function resultResponse(result:ResultEvent):void
  {
    throw new Error( "Abstract operation" );
  }
       
  // Provides default service exception handling
  // universally across all Responder implementations.
  // Concrete implementations can also override this
  // method if specific fault handling needs to be
  // implemented
  public function faultResponse(fault:FaultEvent):void
  {
    // implement default service exception handling
  }
}

We could now refactor the original Responder implementation to the following simplified implementation:

import mx.rpc.events.ResultEvent;
   
public final class SomeResponder extends AbstractRPCResponder
{      
  override public function resultResponse(result:ResultEvent):void
  {
    // modelLocator…
  }
}

As you can see just be pulling up common references and functionality to just two abstractions we can significantly remove redundancy from all Commands and Responders. As such this allows designs to improve dramatically as it allows for the isolation of tests and limits the amount of concrete implementation code developers need to sift through when working with your code.

It is important to understand that a design which is built in part on Cairngorm must still adhere to the same underlying Object Oriented Design principles as any other API would, and in doing so you will end up with a much simpler design which can easily scale over time.