Is this the right way to implement Factory Method design pattern

Question

I have to implement a fee structure different for various franchise depending on certain criteria. i make an Abstract Class for FranchiseFeeStructure

    public abstract class FranchiseFeeStructure {
        private BigDecimal registrationFee=1500;
        protected BigDecimal monthlyFee;

    public BigDecimal getMonthlyFees(){  
            return monthlyFee;  
        }

    public BigDecimal  calculateFee{
        //calculate fee here 
        return fee;
        }
}

Now for different structure i make classes which extends FranchiseFeeStructure like

public class ST1 extends FranchiseFeeStructure{
    @Override
     void getMonthlyFee(){
         monthlyFee=890;
     }

}

and ST2, ST3 and so on for various structures. and we have a class to have static factory method.

public class GetFeeStructureFactory {

    public static  FranchiseFeeStructure getFranchiseFeeStructure(String franchiseFeeStructureType){  

         if(franchiseFeeStructureType == null){  
          return null;  
         }  
       if(franchiseFeeStructureType.equalsIgnoreCase("ST1")) {  
              return new ST1();  
            }   
        else if(franchiseFeeStructureType.equalsIgnoreCase("ST2")){  
    /// so on...

    }
}

Now use it as

FranchiseFeeStructure franchiseFeeStructure = GetFeeStructureFactory.getFranchiseFeeStructure("ST1");
        franchiseFeeStructure.getMonthlyFee();
            franchiseFeeStructure.calculateFee();

Is this the right way to implement the method Factory pattern. Please tell me if i am wrong or any suggestion to make it better.

Answer 1

The problem is not so in the factory implementation. It's in the way your abstract class is implemented. It forces every caller to initialize the monthly fee, by calling a (badly named) method getMonthlyFee(). The caller shouldn't have to do that. And your abstract class should rely... on an abstract method to do that:

public abstract class FranchiseFeeStructure {
    private static final BigDecimal REGISTRATION_FEE = new BigDecimal(1500);

    public final BigDecimal calculateFee {
        BigDecimal monthlyFee = getMonthlyFee();
        // TODO compute
        return fee;
    }

    /**
     * Subclasses must implement this method to tell what their monthly fee is
     */
    protected abstract BigDecimal getMonthlyFee();
}

public class ST1 extends FranchiseFeeStructure {
    @Override
    protected BigDecimal getMonthlyFee(){
        return new BigDecimal(890);
    }
}

That way, the caller doesn't need to initialize the monthly fee. All it needs to do is

BigDecimal fee = FeeStructureFactory.getFranchiseFeeStructure("ST1").calculateFee();

Note that if the monthly fee is always a constant value, you don't even need an abstract class and multiple subclasses. All you need is a single class taking the monthly fee as a constructor argument.