views:

100

answers:

1

I was going through the 'Factory method' pages in SO and had come across this link. And this comment. The example looked as a variant and thought to implement in its original way: to defer instantiation to subclasses...

Here is my attempt. Does the following code implements the Factory pattern of the example specified in the link? Please validate and suggest if this has to undergo any re-factoring.

public class ScheduleTypeFactoryImpl implements ScheduleTypeFactory {

    @Override
    public IScheduleItem createLinearScheduleItem() {
            return new LinearScheduleItem();
    }

    @Override
    public IScheduleItem createVODScheduleItem() {
     return new VODScheduleItem();
    }

}

public class UseScheduleTypeFactory {

    public enum ScheduleTypeEnum {
        CableOnDemandScheduleTypeID, 
            BroadbandScheduleTypeID, 
            LinearCableScheduleTypeID, 
            MobileLinearScheduleTypeID
    }

    public static IScheduleItem getScheduleItem(ScheduleTypeEnum scheduleType) {
        IScheduleItem scheduleItem = null;
        ScheduleTypeFactory scheduleTypeFactory = new ScheduleTypeFactoryImpl();
        switch (scheduleType) {
        case CableOnDemandScheduleTypeID:
            scheduleItem = scheduleTypeFactory.createVODScheduleItem();
            break;

        case BroadbandScheduleTypeID:
            scheduleItem = scheduleTypeFactory.createVODScheduleItem();
            break;

        case LinearCableScheduleTypeID:
            scheduleItem = scheduleTypeFactory.createLinearScheduleItem();
            break;

        case MobileLinearScheduleTypeID:
            scheduleItem = scheduleTypeFactory.createLinearScheduleItem();
            break;
        default:
            break;
        }
        return scheduleItem;
    }
}
+2  A: 

I'd say that it's OK, though overly complex in my opinion. You don't really need the ScheduleTypeFactoryImpl class. Then you can change the UseScheduleTypeFactory class to just ScheduleTypeFactory.

What I'm thinking is this:

public class ScheduleTypeFactory {

public enum ScheduleTypeEnum {
    CableOnDemandScheduleTypeID, 
        BroadbandScheduleTypeID, 
        LinearCableScheduleTypeID, 
        MobileLinearScheduleTypeID
}

public static IScheduleItem getScheduleItem(ScheduleTypeEnum scheduleType) {
    IScheduleItem scheduleItem = null;
    switch (scheduleType) {
    case CableOnDemandScheduleTypeID:
        scheduleItem = new VODScheduleItem();
        break;

    case BroadbandScheduleTypeID:
        scheduleItem = new VODScheduleItem();
        break;

    case LinearCableScheduleTypeID:
        scheduleItem = new LinearScheduleItem();
        break;
    case MobileLinearScheduleTypeID:
        scheduleItem = new LinearScheduleItem();
        break;
    default:
        break;
    }
    return scheduleItem;
}

}

And you might want to include a 'default' case.

aduric