Is a good idea to use the Builder pattern?

[ajnebro]

A positive point of using them is uniformity. In the current implementation of jMetal, to configure an algorithm such as NSGA-II the code looks like this:

    crossover = new SBXCrossover.Builder()
            .setDistributionIndex(20.0)
            .setProbability(0.9)
            .build() ;

    mutation = new PolynomialMutation.Builder()
            .setDistributionIndex(20.0)
            .setProbability(1.0 / problem.getNumberOfVariables())
            .build();

    selection = new BinaryTournament2.Builder()
            .build();

    algorithm = new NSGAIITemplate.Builder(problem, evaluator)
            .setCrossover(crossover)
            .setMutation(mutation)
            .setSelection(selection)
            .setMaxEvaluations(25000)
            .setPopulationSize(100)
            .build(nsgaIIVersion) ;

The positive point is that the pattern allows to use a same mechanism to create and configure operators and algorithms.

[matthieu-vergne]

Personally I would not use a builder for your operators, as they come to be fairly simple (a few arguments). I think it is better to manage them with several constructors or with a factory. You can make one CrossoverFactory which centralizes all your crossover operators (with one or several methods for each of them).

I would keep the builder for the algorithms, which I expect to be more complex. But in a different class, not in the same than the class built (S in SOLID -> single responsibility for each class). Notice that, if you respect the pattern, you should not have:

algorithm = new NSGAIITemplate.Builder(problem, evaluator)
            //...
            .build(nsgaIIVersion) ;

but:

algorithm = new NSGAIITemplate.Builder(problem, evaluator)
            //...
            .setVariant(nsgaIIVersion) ;
            .build() ;

[ajnebro]

I agree with the your last comment.

Anyway, as we are proceeding step by step, my idea is to implement NSGA-II when we have all the basic "ingredients". Then we could see the issue of the Builder pattern.

[ajnebro]

Well, thinking in operators and builders, it's true that que operators usually have one or at most two parameters, so this:

Mutation mutation = new PolynomialMutationOperator() ; // constructor with default values
Mutation mutation2 = new PolynomialMutationOperator(0.1, 5.0) ; // constructor setting the two parameters

is clearly more compact and requires less code into the operator than:

Mutation mutation = new PolynomialMutationOperator.Buiider().build() ; // default
Mutation mutation2 = new PolynomialMutationOperator.Builder() // with settings
  .setProbability(0.1)
  .setDistributionIndex(5.0)
  .build() ;

What I like of the second approach is that it becomes clear which parameters are being set; the alternative of using a constructor is error prone.

In previous versions of jMetal we did some like this:

HastMap<> parameters ;
parameters = new HastTable<String, String> ;
parameters.put<"probability", "0.1"> ;
parameters.put<"distributionIndex, "5.0">;
Mutation mutation = new PolynomialMutationOperator(parameters) ; 

I disliked this approach a lot and removed it for the builder scheme.

I'm re-reading your comments about in #15. A possible alternative could be:

Mutation mutation = new PolynomialMutationOperator() ; // constructor with default values
((PolynomialMutationOperator)mutation).setProbability(0.1) ;
((PolynomialMutationOperator)mutation).setDistributionIndex(0.5) ;

But I still see the builder approach as the most easy one to understand from the point of view of the "user" of the operator.

[matthieu-vergne]

One could tell you that if you think that the multiple constructor alternative is not good because you do not explicit which value goes to which argument (this is implicit via the positioning), then you are just taking the developer for a stupid guy.

I agree that using the builder, you make everything explicit, and due to that I agree with you it is safer. Now probably the feeling above will be the most present. Thus, I agree that the builder alternative add some value regarding the code clarity: you could change the position of the arguments of the constructor without the user noticing it, which would be a problem which does not happen with the builder. Yet, most of the people probably would use (and prefer) the constructor alternative. So as you are making a tool aimed to be user-friendly, I highly recommend you to have the constructor alternative. The builder alternative can also be here if you think it has a significant added value.

[matthieu-vergne]

The point is not to force yourself to remove what you think could be useful in order to fit some standards or to satisfy some "knowledgable guys". Put what you think is useful. Standards and recommendations are here to help people, not contrain them.

[ajnebro]

I agree that, although the Builder approach is pretty attractive to me, there is no reason to not include a constructor in the operators. In fact, given that this is the most common way, the recommendation that could be indicated in the future user manual could be that, when implementing an operator, to provide the constructor and optionally a builder.

For now, I will consider to include both alternatives.

[ajnebro]

After thinking (again) about this issue and taking into account our discussion here I have decided to choose the simplest approach: not using builders in the operators. Thus, an example is:

    crossover = new SBXCrossover(0.9, 20.0) ;

and if we want to enhance the readability then we can use something like this:

    double crossoverProbability = 0.9 ;
    double crossoverDistributionIndex = 20.0 ;
    crossover = new SBXCrossover(crossoverProbability, crossoverDistributionIndex) ;

[ajnebro]

In the case of configuring algorithms, we still use builders, but in a separate class. For example, NSGAII(https://github.com/jMetal/jMetal/blob/master/jmetal-algorithm/src/main/java/org/uma/jmetal/algorithm/multiobjective/nsgaii/NSGAII.java) and NSGAIIBuilder (https://github.com/jMetal/jMetal/blob/master/jmetal-algorithm/src/main/java/org/uma/jmetal/algorithm/multiobjective/nsgaii/NSGAIIBuilder.java).

[matthieu-vergne]

For me, +1 for all of that! Good job {^_^}. Just that I would not make the problem as an argument of the builder's constructor, but a setter like any other parameter. Not only because I don't see why the problem should be considered differently (racist! {>.<}) but also because 0-argument constructor are more generic (cf. the last paragraph).

You could also add the getters in the builder: useful when a builder is used among different methods, each having their own responsibility and deciding what to set depending on what has been already set

You can also make the members of the algorithm final. This is a matter of perspective, but in general nobody think about their object changing its settings while they are using it (excepted for objects designed for that). In such a case, it seems to me that taking the habit to make any member final is a good one. This way, you avoid unexpected changes, and you remove the final only when the member is expected to change during time. A different setting would then be provided through a different object, but with the builder it is extremely easy to manage: use the corresponding setters and the build method to get the new instance, no need to set everything once again, because the values are already stored in the builder.

An incompatible perspective is the JavaBean, which implies to have a 0-argument constructor, so any argument have to be set through a setter, which implies the member to not be final. This is a different way to manage, as it has its own pros and cons. From my perspective, the full constructor with final mambers is safer, while the JavaBean is more standard (0-argument constructors and setters can be used easily with reflexion, allowing a high generalizability). Using the full constructor and final members with a builder is a way to get advantages from both perspectives (the builder provides a JavaBean-like interface).