I had a job to check our project code quality. And have to report it back to my team leader for any obstacle that i found in the project. I found a lot of leaks and i think would be good to be discussed on the blog. Not to mock the author, but to learn and improve ourselves together.

Like this code, this is the part that i found in our code.

 

public ContactInfoBean(final Reseller resellerInfo) {

       switch(resellerInfo.getType()) {

           case PROGRAM_CONTACT:
                   readExecutiveInfo(resellerInfo);
               break;
           case FILE_CONTACT:
                  readOperationalInfo(resellerInfo);
               break;
           default:

               break;

       }

   }

The code works fine, and do its job pretty well. But some problem will appear by using this code-style. This class will grow tailing the biz changes, as usual, the bigger one class, the “merrier” to maintain it is. And most likely this class, will be having more than one purpose, can be called low-cohesion.

Better OOP Approach

Well the better approach for the case above would be using the Factory Design Pattern. We can let the factory of READER to generate every single instance according to their type. It would be easier to grow the instance type, since we just need to create a new class and do a little modification in the Factory class. The caller class, wont grow and will stand still at its current shape.

public interface InfoReader {

      public void readInfo();

}


public class ExecutiveReader implements InfoReader {

      public void readInfo() {
           // override
      }

}

 


public class OperationalReader implements InfoReader {

        public void readInfo() {
         // override
        }

}

And The Factory

public class InfoReaderFactory {

      private static final int PROGRAM_CONTACT = 1;

      private static final int FILE_CONTACT = 2;

      public static InfoReader getInstance(Reseller resellerInfo) {

              InfoReader instance = null;

             switch (resellerInfo.getType()) {

                 case PROGRAM_CONTACT:

                       instance = new ExecutiveReader();

                    break;

                 case FILE_CONTACT:

                     instance = new OperationalReader();

                   break;

                default:

                    throw new IllegalArgumentException('Unknown Reseller');

      }

     return instance;

   }

}

And now The Caller

InfoReader reader = InfoReaderFactory.getInstance(resellerInfo);

reader.readInfo();


The Benefits

With the Factory Design Pattern to handle this case, we can achieve some benefits,

  • Specifying a class for one task, means, easier to maintain since one class is for one purpose only (modularity/High Cohesion). i.e: Operational Reader is only to read data for Operational only, no other purpose. Just in case, one day in the future we need another Reader (say: NonOperationalReader). We just need create a new Class that extends (or implements) the InfoReader class and then we can override our own readInfo() function. This Caller class will have no impact. We just need to do some modification in the Factory code.


public class InfoReaderFactory {
    private static final int PROGRAM_CONTACT = 1;
    private static final int FILE_CONTACT = 2;
    private static final int NEW_READER = 3;

    public static InfoReader getInstance(ResellerInfo resellerInfo) {
        InfoReader instance = null;
        switch (resellerInfo.getType()) {
          case PROGRAM_CONTACT:
              instance = new ExecutiveReader();
              break;
          case FILE_CONTACT:
              instance = new OperationalReader();
              break;
          case NEW_READER:
              instance = new NonOperationalReader();
              break;
          default:
              throw new IllegalArgumentException('Unknown Reseller');
        }
        return instance;
    }
}

Higher Reusability of Parent’s Component (Inheritance): Since we have parent class (InfoReader), we can put common functions and thingies inside this InfoReader class, and later all of the derivative classes (ExecutiveReader and OperationalReader) can reuse the common components from InfoReader . Avoid code redundancy and can minimize coding time. Eventhough this one depends on how you do the code and cant be guaranteed.


But, It’s Run Perfectly, Should We Change It?

Obviously the answer is big NO. This is only the case study and for your further experience and knowledge. OOP is good, do it anywhere it’s applicable. But the most important thing is, if it’s running, dont change it. It would be ridiculous if you ruin the entire working code just to pursue some OOP approach. Dont be naive also, no one can achieve the perfect code. The most important is we know what is the better approach.

 

 

ref:http://namingexception.wordpress.com/2012/10/17/case-study-factory-design-pattern/