2
\$\begingroup\$

I am trying to calculate the print cost base on different paper size, single side or double side. So here is the detail:

A4, job type single sided:
*15 cents - black and white page
*25 cents - color page.

A4, job type double sided:
*10 cents - black and white page
*20 cents - color page.

Also need to support for other paper sizes will be added in the future.

And according to my design, developer can just create a A5 class for example to support other paper size, and add other condition in the factory class.

Could someone review my code and help me on whether I have to use interface instead of abstract class?

Here is my code:

PageBase:

public abstract class PageBase {
    abstract double GetCost(int total, int color, boolean isSingleSide);
    abstract void CalculateUnitPrice(boolean isSingleSide);
}  

A4Page Class:

public class A4Page extends PageBase {
    public double blackAndWhitePrintUnitCost;
    public double colorPrintUniCost;

    @Override
    public double GetCost(int total, int color, boolean isSingleSide) {
        CalculateUnitPrice(isSingleSide);
        return color* colorPrintUniCost + (total-color)* blackAndWhitePrintUnitCost;
    }

    @Override
    public void CalculateUnitPrice(boolean isSingleSide) {
        if (isSingleSide) {
            this.blackAndWhitePrintUnitCost = 0.15;
            this.colorPrintUniCost = 0.25;
        }
        else {
            this.blackAndWhitePrintUnitCost = 0.10;
            this.colorPrintUniCost = 0.20;
        }
    }
}  

PageFactory:

public class PageFactory {

    public PageBase GetPage(String pageType) {
        switch (pageType.toUpperCase()) {
            case "A4":
                return new A4Page();
            default:
                return new A4Page();
        }
    }
}

Main:

public class Main {
    public static void Main() {
        //read
        PageFactory pageFactory = new PageFactory();
        PageBase page = pageFactory.GetPage("A4");
        page.GetCost(0,0,false);
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Design

you should consider to define the properties of the page in factory, that's where it should be located.

public PageBase GetPage(String pageType) {
    switch (pageType.toUpperCase()) {
        case "A4": return new Page(0.15, 0.25, 0.10, 0.20);
}

Programming

1. Page

you should not try to subclass the page since it is not required, the only difference between each page are the prices (so far). Therefore you don't need subclasses - it is just a page (not a base Page or anything else)

And even more you should could instanciate the class with all properties during construction (as shown above, dependency injection)

public class Page {

    private final double blackWhiteSinglePageUnitCost;
    private final double colorSinglePageUnitCost;
    private final double blackWhiteDoublePageUnitCost;
    private final double colorDoublePageUnitCost;

    public Page(double bwSingle, double colorSingle, double bwDouble, double colorDouble){
        blackWhiteSinglePageUnitCost= bwSingle;
        colorSinglePageUnitCost= bwDouble;
        blackWhiteDoublePageUnitCost= bwSingle;
        colorDoublePageUnitCost= colorDouble;
    } 

what can be kept ist GetCost but should be renamed int getCost() but you can totally skip CalculateUnitPrice, see below

2. PageType

if you use a certain format you should use Enums to define the Format of the sheet. that would free you from abusing Strings as datatypes

enum PageType {A4, A5, LETTER }; 

3. bringing things together

when we use all these hints from above we have to adjust some more code, to make the programm run again

Page.class must be modified we can calculate the price on-the-fly because we already know anything required to calculate the price:

public double getCost(int total, int color, boolean isSingleSided){
    int bw = total - color;
    if(isSingleSided){
        return bw * blackWhiteSinglePageUnitCost + color * colorSinglePageUnitCost ;
    else{
        return bw * blackWhiteDoublePageUnitCost + color * colorDoublePageUnitCost ;
    }

}

PageFactorymust be modified - first the method signature must be java conform, namely getPage not GetPage. then you switch over the enum type instead of an String. That lets you handle invalid inputs.

public Page getPage(PageType pageType) { 
    switch (pageType) {
        case PageType.A4:
            return ...;   
        case PageType.A5:
            return ...;   
        case PageType.LETTER:
            return ...;            
    }
}

if you use this in action (in your main method ) it would be finally that appearance:

public class Main {
    public static void Main() {
        PageFactory pageFactory = new PageFactory();
        Page page = pageFactory.getPage(PageType.A4);
        double cost = page.getCost(0,0,false);
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.