2

I'm creating a library, and I want there to be an abstract class called Optimizer. The user should be able to choose from a set of predefined Optimizers and if desired, use fluent methods to change certain fields.

Every Optimizer has a "learning rate", so I've defined learningRate as a field within that class as well as its fluent setter.

abstract class Optimizer {
    private double learningRate;

    public Optimizer withLearningRate(double learningRate) {
        this.learningRate = learningRate;
        return this;
    }
}

One type of Optimizer should be "Gradient Descent" (which has a default learning rate of 0.01 + more properties than just learning rate), so I defined OptimizerGD accordingly:

public class OptimizerGD extends Optimizer {
    private double momentum = 0; // default momentum
    private boolean nesterov = false; // default nesterov

    public OptimizerGD() {
        withLearningRate(0.01); // default learning rate
    }

    public OptimizerGD withMomentum(double momentum) {
        this.momentum = momentum;
        return this;
    }

    public OptimizerGD withNesterov() {
        this.nesterov = true;
        return this;
    }
}

And I want the user to be able to select this Optimizer in a simple way, like Optimizer.GD, so I added this line to my Optimizer class:

public static final OptimizerGD GD = new OptimizerGD();

Allowing for:

model.compile().withOptimizer(GD.withNesterov());


Here are my concerns with my current design pattern, and any ideas I have for possible solutions:

  • IntelliJ warns me that "Referencing subclass OptimizerGD from superclass Optimizer initializer might lead to class loading deadlock".

My idea for this is to make another class called Optimizers to contain the static field for GD (and other ones I add later), although I think I would prefer to keep this stuff in Optimizer because that seems more conventional? I suppose this doesn't make too much of a difference from the user's POV though.

  • My fluent setter for learning rate returns Optimizer instead of the specific subtype of Optimizer, meaning that it always has to be called last when I chain setters (GD.withNesterov().withLearningRate(0.5))

My idea for this is to remove withLearningRate from Optimizer, and instead have it implemented in each subclass I make so that it can return the subtype. The reason I hesitate to embrace this is because I'm repeating the same code with just a different return type.

  • I don't want the user to be able to implement their own Optimizer or instantiate OptimizerGD. I want them to always use GD followed by any setters they may wish to use. I believe I've prevented Optimizer implementations by making it package-private, but OptimizerGD has to be public, it seems, for the setters to be accessible.

Don't know what to do here. Maybe my design pattern needs to use enums?

I've asked several things here but they're all connected to my one main question, which is what the design pattern of my code should be given my goals and specifications.

3

The idea of providing a static instance of OptimizerGD (which is just a singleton in disguise) looks flawed, since optimizers are stateful. Assume one wants one OptimizerGD object "without Nesterov", and a second one "with Nesterov" - in the current design, this would not be possible, since there would always be only one OptimizerGD object with either nesterov set to true or to false.

I don't want the user to be able to [...] instantiate OptimizerGD

But why? I think if I would have to implement this, I would actually want the user to do exactly this - create individual Optimizer (or OptimizerGD) objects with individual settings (and so remove the necessity of having a static instance Optimizers.GD at all). That would solve two of your three issues just by removing overcomplicated stuff.

For the problem of mixing inheritance with fluent interface, I found this short article. It shows how to utilize generics for solving the problem. Applied to your case, this results in something along the lines of

public class Optimizer<T extends Optimizer<?>> {

  protected final T self;

  protected Optimizer(final Class<T> selfClass) {
    this.self = selfClass.cast(this);
  }


   public T withLearningRate(double learningRate) {
      this.learningRate = learningRate;
      return self;
   }
}

However, an even simpler solution to this could be sticking to the KISS principle and not to use a fluent interface at all. Sure, the API then may look a little bit more "conventional", maybe "boring", but think twice if you are sure the "fluent interface" is really important for the users of your API, or if they may also almost equally be happy with a non-fluent one.

  • Good old F-bounded polymorphism. Is there any problem, you can't solve? :-D – Jörg W Mittag Oct 27 '19 at 19:18
  • - in the current design, this would not be possible, since there would always be only one OptimizerGD object with either nesterov set to true or to false Yikes! Didn't realize this – K_7 Oct 27 '19 at 19:46
  • Resolved my issues by 1. implementing the fluent-inheritance pattern 2. realizing that users should instantiate a new instance of the optimizer if they want to change some of the default parameters – K_7 Oct 27 '19 at 22:59
  • My only outstanding question is how to prevent users from calling the setters on Optimizers.GD? ie. new Gd().withNesterov is fine, but GD.withNesterov() isn't. I'm thinking I'll make a second constructor that takes a boolean, and as long as this boolean is true, then no state changes can occur within the Optimizer. And then define GD as the following public static final Gd GD = new Gd(true); – K_7 Oct 27 '19 at 23:20
  • 1
    @K_7: ... however, the fluent interface offers another alternative to this: making your Optimizer objects immutable. That would mean there were no public methods like setters which can change the state of an object after its construction, and methods like withLearningRate or withNesterov will always return a new Optimizer object instead of a modified reference to the original one. But beware, immutability and inheritance don't mix well, I would be hesitant with such a design. – Doc Brown Oct 28 '19 at 8:30
1

Abstract classes should not depend on concrete classes. Furthermore, this would be against the open/close principle: every time you'd create a new kind of optimizer, you'd need to update the base class to create a new static member.

Isolating these static optimizers in a separate class would decouple the things a little more. But you would have to create optimizers without even being sure that you need them.

So there are two possibilities:

  • either you let the user instantiate the desired optimizer;
  • or you use a Factory to instantiate the optimizer.

According to the DRY principle, it would not be such a nice idea to redefine a common setter in every subclass, doing the same thing again and again, just for the sake of having a covariant return type for method chaining. On the other hand, keeping it as it is, forces users to remember which setter is common and which is specific. This goes against the principle of the least knowledge.

So, if whatever you do there are drawbacks, are the chained setters really worth the additional constraints ? I'd recommend not to use them to keep it simple.

  • Thanks for the answer, but I'm set on keeping static members of some sort. The 'coupling' that exists is somewhat non-unique because the optimizers have to be coupled with the internal library code anyways (if its a gradient descent optimizer, do XYZ, if its a different optimizer, do something else, etc). I also am set on fluent as its a main characteristic of my library. – K_7 Oct 27 '19 at 23:09
  • My main mistake in all of this was not realizing that calling setters on a static member changes its state, so the user should instantiate a new optimizer if they want to induce state changes. – K_7 Oct 27 '19 at 23:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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