6

According to https://softwareengineering.stackexchange.com/a/334994/432039, I know init is a code smell and should be avoided, and one of the solutions is to use a builder to hold the state first instead of create the object first and then set the state, like https://softwareengineering.stackexchange.com/a/452495/432039 mentions, eg:

With init:

public class MyView extends View{
    public MyLabel myLabel;
    public void init(MyLabel myLabel){
        this.myLabel=myLabel;
    }
} 
.
.
.
MyLabel myLabel=new MyLabel();
.
.
.
MyView myView=new MyView();
//some other code
myView.init(myLabel);

Without init:

public class MyViewBuilder{
    private MyView _myView;
    public void setMyLabel(MyLabel myLabel){
        this._myView.myLabel=myLabel;
    }
    public MyView build(){
        return _myView;
    }
}
.
.
.
MyLabel myLabel=new MyLabel();
.
.
.
MyViewBuilder myViewBuilder=new MyViewBuilder();
myViewBuilder.setMyLabel(myLabel);
MyView myView=myViewBuilder.build();

However, how to eliminate init when 2 objects need to reference to each other? For example, there are 2 UI Views: MyView and MyLabel, which MyView has a child view MyLabel, but MyLabel also needs to keep the reference of MyView as parent view, eg:

public class MyView extends View{
    public MyLabel myLabel;
    public void init(MyLabel myLabel){
        this.myLabel=myLabel;
    }
} 

public class MyLabel extends Label{
    public View parentView;
    public void init(View parentView){
        this.parentView=view;
    }
} 

MyLabel myLabel=new MyLabel();
MyView myView=new MyView();
myView.init(myLabel);
myLabel.init(myView);

I expect the modified code would look like:

MyViewBuilder myViewBuilder=new MyViewBuilder();
myViewBuilder.setMyLabel(myLabel);
MyView myView=myViewBuilder.build();
.
.
.
MyLabelBuilder myLabelBuilder=new MyLabelBuilder();
myLabelBuilder.setParentView(myView);
MyLabel myLabel=myLabelBuilder.build();

but it seems not possible : it must at least either one side already created first:

MyLabel myLabel=new MyLabel();
.
.
.
MyViewBuilder myViewBuilder=new MyViewBuilder();
myViewBuilder.setMyLabel(myLabel);
MyView myView=myViewBuilder.build();

myLabel.init(myView);

Which the pre-init state seems unavoidable. Is it true? If not, how can I avoid init in this case?

Note: the init code is just simplified from real business code, which MyView may contain other UI elements, and the init code can't refactor as follows:

public class MyView extends View{
    public MyLabel myLabel;
    public MyView(MyLabel myLabel){
        this.myLabel=myLabel;
    }
} 

Because the MyView needs to create at the start of application, but show later, and there are some other UI elements in MyView that have blink animation, those blink animation needs to sync with other elements with blink animation outside MyView,eg:

public class MyAppDelegate extends MyAppDelegate{
    public MyAppDelegate(){
        .
        .
        .
        GlobalViews.myView=new MyView();
        MyView myView=GlobalViews.myView;
        myView.badge.playBlinkAnimationForever();
        //some other UI
        GlobalViews.myTabBarView=new MyTabBarView();
        MyTabBarView myTabBarView=GlobalViews.myTabBarView;
        myTabBarView.playBlinkAnimationForever();
    }
}

public class LoginView extends View{
    public void onLoginButtonPressed(){
        httpHandler.callHttp("http://some url",new Runnable(){
            public void run(){
                .
                .
                .
                MyView myView=GlobalViews.myView();
                if(response.isGoldenMember){
                    myView.init(new MyGoldenShinyLabel(response.userName));
                }else{
                    myView.init(new MyLabel(response.userName));
                }
            }
        });
    }
}
5
  • 3
    Thinks that you “know” are code smells are often unjustified opinions given with confidence and certainty and no doubt, and repeated blindly. Just because someone sounds confident doesn’t mean they are right.
    – gnasher729
    Commented Jun 21 at 10:46
  • 7
    "According to [some answer of Doc Brown], I know init is a code smell" - hm, as far as I can see, what I wrote in this answer is "there are cases where an init method is fine".
    – Doc Brown
    Commented Jun 21 at 12:46
  • 2
    ... still, what I don't understand is what's wrong with the quite trivial solution of constructing "label" in the view's constructor, passing "this" as a parameter to the label's constructor, as shown in this answer (2nd example)? Please clarify.
    – Doc Brown
    Commented Jun 21 at 13:39
  • 1
    The first sentence in that linked answer (well, after "It depends") is "An init method is a code smell when it is not necessary to have the object initialization separated from the constructor.". In your case, if you have two objects with an explicit dependency on each other, then you need an init method to facilitate that, which means that, according to that answer's logic, you do not have code smell. (Of course, this is ignoring the debate over whether it's code smell to have two objects with an explicit dependency on each other in the first place.)
    – Abion47
    Commented Jun 21 at 23:18
  • 1
    Duplicate of softwareengineering.stackexchange.com/q/368237/106122
    – Basilevs
    Commented Jun 25 at 5:28

4 Answers 4

8

However, how to eliminate init when 2 objects need to reference to each other? For example, there are 2 UI Views: MyView and MyLabel, which MyView has a child view MyLabel, but MyLabel also needs to keep the reference of MyView as parent view

In my experience, when you think there is such a strong mutual dependency at construction time of the objects, quite often there is a wrong mental model involved of what states of the software are valid.

While a GUI View object with no elements in it is quite useless, it is actually a valid state of the software and can be handy to have temporarily every once in a while. For example, it can easily solve your dependency problem:

public class MyView extends View{
    public MyLabel myLabel;
    public void set_label(MyLabel myLabel){
        this.myLabel=myLabel;
    }
} 

public class MyLabel extends Label{
    public View parentView;
    public MyLabel(View parentView){
        this.parentView=view;
    }
} 

MyView myView=new MyView();
// some other code
MyLabel myLabel=new MyLabel(myView);
myView.add_label(myLabel);
// or even: myView.add_label(new MyLabel(myView));

If there really is a strong need to have the mutual references in place as soon as the objects are constructed, you can use the Factory pattern:

public class MyView extends View{
    public MyLabel myLabel;
    public void MyView(MyLabelFactory myFactory){
        this.myLabel=myFactory.create_label(this);
    }
} 

public class MyLabel extends Label{
    public View parentView;
    public MyLabel(View parentView){
        this.parentView=view;
    }
} 

public class MyLabelFactory extends Object {
    public MyLabel create_label(View parentView)
    {
        return new MyLabel(parentView);
    }
}

MyLableFactory myFactory = new MyLabelFactory();

MyView myView=new MyView(myFactory);

This makes use of the fact that as soon as the constructor starts, the object has been constructed far enough that you can pass its reference on to other objects for storing it there.

3

Another option to consider is whether MyLabel needs to exist outside of MyView at all. Sometimes it is best to simply create the object where it is needed.

public class MyView {
    private final MyLabel label;

    public MyView() {
        label = new MyLabel(this);
    }
}

The instance of MyLabel becomes an implementation detail of MyView. If code outside of MyView needs that label, then add a getter to return it.

You've worked yourself into a chicken or the egg paradox, so you simply need to brute-force decide which one comes first: the chicken (MyView) or the egg (MyLabel).

With all the talk about dependency injection and inversion of control, we sometimes forget that it is perfectly acceptable to initialize an object in a constructor rather than pass it in. Dependency injection is not a law. It is a guideline, and this seems like a perfectly fine reason to violate the guideline.

The downside is that MyView needs to know the dependencies of MyLabel, but this is not a problem if MyLabel has no dependencies (or very few dependencies). I would start with initializing MyLabel in the constructor for MyView. Only after the object construction needs of MyLabel have become sufficiently complex would I consider creating a factory, like Bart's answer illustrates.

If you need additional information pertinent to the label, you can expose methods in MyView that ultimately call methods on MyLabel. For example, consider setting the label text. The easiest way would be adding a constructor parameter, which might not be the cleanest way to do it:

public class MyView {
    private final MyLabel label;

    public MyView(String labelText) {
        label = new MyLabel(this);
        label.setText(labelText);
    }
}

From the perspective of the caller, your code becomes:

MyView view = new MyView("Label text goes here");

Or allow callers to set the label text via MyView:

public class MyView {
    private final MyLabel label;

    public MyView() {
        label = new MyLabel(this);
    }

    public void setLabelText(String labelText) {
        label.setText(labelText);
    }
}

Now callers have an additional method on MyView:

MyView view = new MyView();

view.setLabelText("Label text goes here");
2
  • And you shouldn’t need the label. Or in a system with many different UI elements, you shouldn’t need to know that it is a Label and not some other class. So MyView should be able to use a different class if that is preferred.
    – gnasher729
    Commented Jun 21 at 16:27
  • True. The label field could just be a View and the only reference to MyLabel would be on the constructor of MyView. Commented Jun 21 at 16:50
1

There are too many options. Without considering design patterns and object oriented principles, the way SOLID principles are, the solution is to pass a view to the label's constructor and in the label's constructor set the label into the view. According to the description there isn't any impediment in doing it that simple.

. . . 

MyLabel label = new MyLabel(new MyView());
MyView view = label.view;

. . .

public class MyLabel {

    public final View view;

    public MyLabel(View view) {
        this.view = label(view);
    }

    private View label(View view) {
        view.setLabel(this);
        return view;
    }
}

When the quest is for a solution using immutable fields in both view and label the implementation should use constructor initialisers (a code snippet that is called before running the invoked constructor). Constructor initialisers offer the benefit of running complex code blocks including conditional statements, loops and try/catch statements:

. . .

MyView view = new MyView();
MyLabel label = view.label;

. . .

public class MyView {
    public final MyLabel label;
    {
        this.label = new MyLabel(this);
    }
}

public class MyLabel {

    public final View view;

    public MyLabel(View view) {
        this.view = view;
    }
}

To get rid of init method in a fancy way just figure a fancy design pattern to implement: factory and builder or visitor or observer. Bottom line is: it is just a composition, choosing the design pattern to implement it is equivalent to choosing the syntactic sugar to spice it up.

With visitor design pattern the implementation could be...

interface Visitor {
    void visit(MyView visited);
}
interface Visited {
    void accept(MyLabel visitor);
}
class MyView implements Visited {

    private MyLabel label;

    public MyView() {}

    public void accept(MyLabel visitor) {
        this.label = visitor;
        visitor.visit(this);
    }

    public MyLabel getLabel() { return this.label; }
}

class MyLabel implements Visitor {
    
    private MyView view;

    public MyLabel() {}

    public void visit(MyView visited) {
        this.view = visited;
    }
  
    public MyView getView() { return this.view; }
}

Note: there are just getters provided to have the composition achievable through visit...

. . .

MyView view = new MyView();
view.accept(new MyLabel());

MyLabel label = view.getLabel();

. . .

With observer design pattern the initialisation could be...

MyView myView = new MyView(new MyLabel()).update();
MyLabel myLabel = myView.getLabel();

...when the the design pattern has following implementation...

interface Observable<R extends Observer> {

    boolean register(R observer);

    boolean withdraw(R observer);

    <R extends Observable> R update();
}

interface Observer<T extends Observable> { void observe(T observable); }

class View<T extends Observer> implements Observable<T> {

    private T label;

    public View() {}

    public boolean register(T observer) {
        if ( this.label != null || observer == null ) { return false; }
        this.label = observer;
        return true;
    }

    public boolean withdraw(T observer) { 
        if ( this.label != observer ) { return false; }
        this.label = null;
        return true;
    }

    @Override
    public View update() {
        this.label.observe(this);
        return this;
    }

    public T getLabel() { return this.label; }
}

class MyView extends View<MyLabel> {

    public MyView(MyLabel label) {
        super.register(label);
    }

    @Override
    public MyView update() {
        super.update();
        return this;
    }
}

class Label<T extends View> implements Observer<T> {
    
    private T view;

    public void observe(T observable) {
        this.view = observable;
    }
}

class MyLabel extends Label<MyView> {}

The observer design pattern is different that the Observer provided by Java API that it is deprecated starting with Java version 9.

I omitted the implementation using builder and factory design patterns since there are plenty of examples available.

-1

Just ignore what you heard about “code smells”.

Just create a subclass of View. And give it an init method that creates a complete instance: Init the baseclass, create all the subviews, make them accessible or better, make their meaning accessible (nobody should care that there is a label, only that some text is displayed), connect all the sub views inside the view and you are done.

For example if you want two text entries for first name and last name together with a label “First name” and “Last name”, your unit has firstNameText and lastNameText as arguments, plus a “Name” or a “Person” object, and creates two labels, two editable fields, and possibly a Cancel or Save button if the user can edit the name.

Anything else just makes your code pointlessly complicated.

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