How to handle the Duplicated blocks

Question

I ran sonarqube, but it informed me the error

"Duplicated blocks" in class Gold & Silver.

I modified many things, but it didn't solve the problem

Here is my source

Gold and Silver is extends Plan

Here is a Plan.Source

public abstract class Plan {
public abstract double getBasicRate();
public abstract int getBasicMinute();
public abstract double getAdditionalLineRate();
public abstract double getRatePerExcessMinute();
public abstract String getPlanName();}

Here is Gold.Source

public class Gold extends Plan {

private static final double BASIC_RATE = 49.95;
private static final int BASIC_MINUTE = 1000;
private static final double ADDITIONAL_LINE_RATE = 14.50;
private static final double RATE_PER_EXCESS_MINUTE = 0.45;
private static final String PLAN_NAME = "Gold";

public double getBasicRate() {
    return BASIC_RATE;
}

public int getBasicMinute() {
    return BASIC_MINUTE;
}

public double getAdditionalLineRate() {
    return ADDITIONAL_LINE_RATE;
}

public double getRatePerExcessMinute() {
    return RATE_PER_EXCESS_MINUTE;
}

public String getPlanName() {
    return PLAN_NAME;
}

}

Here is a Silver.Source

public class Silver extends Plan {

private static final double BASIC_RATE = 29.95;
private static final int BASIC_MINUTE = 500;
private static final double ADDITIONAL_LINE_RATE = 21.50;
private static final double RATE_PER_EXCESS_MINUTE = 0.54;
private static final String PLAN_NAME = "Silver";

public double getBasicRate() {
    return BASIC_RATE;
}

public int getBasicMinute() {
    return BASIC_MINUTE;
}

public double getAdditionalLineRate() {
    return ADDITIONAL_LINE_RATE;
}

public double getRatePerExcessMinute() {
    return RATE_PER_EXCESS_MINUTE;
}

public String getPlanName() {
    return PLAN_NAME;
}

}

please help me

unfortunately I don't see a good way to avoid the code dublication — Dimitrios Begnis, Apr 01 '16 at 10:44

score 2 · Accepted Answer · edited May 23 '17 at 12:15

The code in Gold and Silver is identical, except from the data being assigned to the variables. You could refactor to something like this to remove duplication:

public class Plan {

    private final double BASIC_RATE;
    private final int BASIC_MINUTE;
    private final double ADDITIONAL_LINE_RATE;
    private final double RATE_PER_EXCESS_MINUTE;
    private final String PLAN_NAME;

    public Plan(double BASIC_RATE, int BASIC_MINUTE,
            double ADDITIONAL_LINE_RATE, double RATE_PER_EXCESS_MINUTE,
            String PLAN_NAME) {
        this.BASIC_RATE = BASIC_RATE;
        this.BASIC_MINUTE = BASIC_MINUTE;
        this.ADDITIONAL_LINE_RATE = ADDITIONAL_LINE_RATE;
        this.RATE_PER_EXCESS_MINUTE = RATE_PER_EXCESS_MINUTE;
        this.PLAN_NAME = PLAN_NAME;
    }

    public double getBasicRate() {
        return BASIC_RATE;
    }

    public int getBasicMinute() {
        return BASIC_MINUTE;
    }

    public double getAdditionalLineRate() {
        return ADDITIONAL_LINE_RATE;
    }

    public double getRatePerExcessMinute() {
        return RATE_PER_EXCESS_MINUTE;
    }

    public String getPlanName() {
        return PLAN_NAME;
    }
}

Then Gold would look something like this:

public class Gold extends Plan {

    public Gold() {
        super(49.95, 1000, 14.50, 0.45, "Gold");
    }
}

What we have done here is take code that is shared by 2 classes and moved it up into its parent class. By calling the super constructor in Gold we assign the variables to the values required by this implementation of the super class Plan.

We've also removed the static from the class variables. This means that the variables will be related to an instance of the class rather than the class itself.
We do this so that the variables in Plan will be related to each instance of Plan. This means we can use them in both the extending classes without data being mixed between.
In a lot of cases you wouldn't usually use static unless you actively wanted a variable to be accessible without a class instance. You can read more about static here.

that will not work ! because the variables are static, all Plans (Gold and Silver) will have the same value. — Dimitrios Begnis, Apr 01 '16 at 10:47
Oh! Duh... Completely missed that! Thanks. Well then your comment on the question is correct unless OP changes that. — Tiz, Apr 01 '16 at 10:51
@Dimi Fixed it, and added a bit of an explanation why. Thanks! — Tiz, Apr 01 '16 at 10:57

score 1 · Answer 2 · answered Aug 06 '20 at 12:17

I had this issue recently. Although you already have an accepted solution (and it's been 4 years), I would like to share another way.

I prefer to keep these constants as I find it more readable. A (soooo) simple solution was to rename the constant names ;) (i.e - prefixing them with SILVER_ or GOLD_).

KISS principle and it worked perfectly.

public class Gold extends Plan {

private static final double GOLD_BASIC_RATE = 49.95;
private static final int GOLD_BASIC_MINUTE = 1000;
private static final double GOLD_ADDITIONAL_LINE_RATE = 14.50;
private static final double GOLD_RATE_PER_EXCESS_MINUTE = 0.45;
private static final String GOLD_PLAN_NAME = "Gold";

public double getBasicRate() {
    return GOLD_BASIC_RATE;
}

public int getBasicMinute() {
return GOLD_BASIC_MINUTE;
}

public double getAdditionalLineRate() {
    return GOLD_ADDITIONAL_LINE_RATE;
}

public double getRatePerExcessMinute() {
    return GOLD_RATE_PER_EXCESS_MINUTE;
}

public String getPlanName() {
    return GOLD_PLAN_NAME;
}

Adriaan Koster · Answer 3 · 2016-04-01T11:00:14.303

If you really want to have a separate Gold and Silver class like this you can't avoid the duplicate code blocks. You could suppress the Sonar violations, either by adding the following line above the line causing the violation (see Sonar FAQ):

// NOSONAR

Alternatively you could use an enum to represent these values:

public enum Plan {

    GOLD(49.95, 1000, 14.50, 0.45, "Gold"),
    SILVER(29.95, 500, 21.50, 0.54, "Silver");

    private double basicRate;
    private int basicMinute;
    private double additionalLineRate;
    private double ratePerExcessMinute;
    private String planName;

    private Plan(double basicRate, int basicMinute, double additionalLineRate, double ratePerExcessMinute, String planName) {
        this.basicRate = basicRate;
        this.basicMinute = basicMinute;
        this.additionalLineRate = additionalLineRate;
        this.ratePerExcessMinute = ratePerExcessMinute;
        this.planName = planName;
    }

    public double getBasicRate() {
        return basicRate;
    }
    public int getBasicMinute() {
        return basicMinute;
    }
    public double getAdditionalLineRate() {
        return additionalLineRate;
    }
    public double getRatePerExcessMinute() {
        return ratePerExcessMinute;
    }
    public  String getPlanName() {
        return planName;
    }
}

This also avoids duplicated blocks but might not suit your needs.

How to handle the Duplicated blocks

Here is a Plan.Source

Here is Gold.Source

Here is a Silver.Source

3 Answers3