CODEWITHSUNDEEP
javaclassoopabstract-class
Shepard62FR
Shepard62FR

Reputation: 300

Creating childs of an abstract class based on childs of another abstract class without knowing their type

Consider the following situation:

When I'm creating the statistics for my client, I'm doing something rather ugly which is this:

CaracteristicA caracteristicA = null;
CaracteristicB caracteristicB = null;
CaracteristicC caracteristicC = null;

for (Caracteristic currentCaracteristic : listOfCaracteristics) {
    if (currentCaracteristic instanceof CaracteristicA) {
        caracteristicA = (CaracteristicA)currentCaracteristic;
    } if (currentCaracteristic instanceof CaracteristicB) {
        caracteristicB = (CaracteristicB)currentCaracteristic;
    } if (currentCaracteristic instanceof CaracteristicA) {
        caracteristicC = (CaracteristicC)currentCaracteristic;
    } else {
        throw new IllegalArgumentException("Unknown caracteristic !");
    }
}

// "client.addStatistic" expects a "Statistic".
// "StatisticA"'s constructor expects an instance of "caracteristicA" and "caracteristicB".
// "StatisticB"'s constructor expects an instance of "caractersiticC".
client.addStatistic(new StatisticA(caracteristicA, caracteristicB));
client.addStatistic(new StatisticB(caracteristicC));

The problem: if I want to add a StatisticC that depends on CaracteristicA and a new child of Caracteristic (let's call it CaracteristicD), I would need to add a variable to store the appropriate caracteristic, add another else if in the loop to find it and then I'm finally able to create the needed statistic with the proper caracteristic.

I'm fairly certain that I can simply this whole mess and I would like to know how I could do that. Searching on Google and Stack Overflow seems to point me towards the "Factory" and "Visitor" design patterns but I'm not sure if they would help me or make it worse.

I also tried adding a Object getType() abstract method to Caracteristic where childrens would override it with their type (example: CaracteristicA getType()) but I realized this would have worked if I knew in advance what child of listOfCaracteristics is being iterated.

UPDATE: listOfCaracteristics is populated by doing listOfCaracteristics.add(new CaracteristicA());, listOfCaracteristics.add(new CaracteristicB()); and so on. Here's the code of the abstract class Caracteristic:

abstract class Caracteristic {
    private final String name;
    private int level;

    Caracteristique(String name, int level) {
        this.name = name;
        this.level = level;
    }

    public int getLevel() {
        return this.level;
    }

    @Override
    public String toString() {
        return this.name;
    }
}

And here's the code of one of it's child (CaracteristicA):

class CaracteristicA extends Caracteristic {
    public CaracteristicA(int level) {
        super("Hello World!", level);
    }

    public CaracteristicA() {
        super("Hello World!", 1);
    }
}

Upvotes: 1

Views: 61

Answers (2)

Jul10
Jul10

Reputation: 503

Maybe I'm wrong because I'm not very sure to understand the question, but why not use Composite Pattern to represent the hierarchy of Carateristic? In this way, you'll be able to create a new Composite charateristic (e.g CaracteristicAB ) and you can pass it to the Statistic that needs more than one Carateristic.

Upvotes: 0

Spotted
Spotted

Reputation: 4091

Instead of creating one instance of each Caracteristic and putting them in the list to afterward decompose the list and assign them to variable, just instanciate the variables with the correct Caracteristic. This drastically simplifies the code:

CaracteristicA caracteristicA = new CaracteristicA();
CaracteristicB caracteristicB = new CaracteristicB();
CaracteristicC caracteristicC = new CaracteristicC();
CaracteristicD caracteristicD = new CaracteristicD(); //hypothetic
client.addStatistic(new StatisticA(caracteristicA, caracteristicB));
client.addStatistic(new StatisticB(caracteristicC));
client.addStatistic(new StatisticC(caracteristicA, caracteristicD)); //hypothetic

There is something more that I want to add. The fact that Caracteristic is an abstract class but has no member abstract is a sign of a bad design. In the end I suspect the only thing you need is a control over which pair of values (a name and a level) are allowed in Statistic. The whole Caracteristic hierarchy could then be simplified with

public enum Caracteristic {
    A("Hello World!", 1), B("easy", 2), C("medium", 3), D("hard", 4);

    private final String name;
    private final int level;
    Caracteristic(String name, int level)
    {
        this.name = name;
        this.level = level;
    }
    @Override
    public String toString() { return name; }
    public int getLevel() { return level; }
}

Which simplify your code again in something like this:

client.addStatistic(new StatisticA(Caracteristic.A, Caracteristic.B));
client.addStatistic(new StatisticB(Caracteristic.C));
client.addStatistic(new StatisticC(Caracteristic.A, Caracteristic.D)); //hypothetic

Now one have really no benefit to take Caracteristic as constructor argument because there exists only one instance of Caracteristic.A so they can be directly used inside StatisticX !

Again an even more simplified version would become:

client.addStatistic(new StatisticA());
client.addStatistic(new StatisticB());
client.addStatistic(new StatisticC()); //hypothetic

With, for example, StatisticA beeing

public final class StatisticA {
    public void DoSomeWork() {
        int differenceLevel = Caracteristic.B.getLevel() - Caracteristic.A.getLevel();
        string bothLevels = Caracteristic.A.toString() + " " + Caracteristic.B.toString();
        System.out.PrintLn(bothLevels + ": " + differenceLevel);
    }
}

To make even one more step, I will make the assumption that each Statistic does the "same" work (except that it uses different caracteristics). This would allow to refactor into one single class Statistic

public final class Statistic {
    private final Iterable<Integer> caracs;
    public Statistic(Caracteristic... caracs) {
        this.caracs = Arrays.asList(caracs);
    }
    public void DoSomeWork() {
        System.out.PrintLn(caracs.stream()
                                 .map(Caracteristic::toString)
                                 .Collect(Collectors.joining(",")));
        System.out.PrintLn(caracs.stream()
                                 .map(Caracteristic::getLevel)
                                 .sum());
    }
}

With usage becoming (ultimate code simplification)

client.addStatistic(new Statistic(Caracteristic.A, Caracteristic.B));
client.addStatistic(new Statistic(Caracteristic.C));
client.addStatistic(new Statistic(Caracteristic.A, Caracteristic.D)); //hypothetic

In the beginning one would have:

  • 7 classes

With all the simplifications:

  • 1 class
  • 1 enum

Upvotes: 1

Related Questions