0

According to https://softwareengineering.stackexchange.com/a/334994/432039, I know "init()" method is a code smell, and "physically make them two separate classes" is a way to refactor the "init()".

However, I don't understand what is the meaning of "physically make them two separate classes". Is it mean move the init method to a separate class? eg: From

public class Demo {
    public Demo(){
    }
    public void init() {
        //...
   }
}

to

public class Demo {
    public Demo(){
    }
}

public class DemoHelper {
    public static void init(Demo demo) {
        //...
   }
}

? If so, would it suffer from "feature envy"?

3 Answers 3

11

I believe the most important part of that answer is:

Here you have an object in two fundamentally different states: pre-init and post-init. Those states have different responsibilities, different methods that are allowed to be called, and different behavior. It's effectively two different classes.

So I believe they were suggesting basically splitting Demo into DemoBuilder and Demo.

The constructor belongs to DemoBuilder, and so does the init method, but instead of returning void and modifying this in place, it will return a Demo object, completely initialised.

Calling code that used to look like this:

Demo d1 = new Demo(...);
// [potential setup code using d1]
d1.init();
// [now d1 can be used normally]

... should then be changed to look like this:

DemoBuilder builder = new DemoBuilder(...);
// [potential setup code using builder]
Demo d1 = builder.build();
// [now d1 can be used normally]

This prevents uninitialised use of your objects, since DemoBuilder simply does not have the same methods as Demo does.

1
  • An alternative to a second Setup or Builder class, if that feels too heavyweight, is a static method.
    – Bergi
    Commented Mar 18 at 15:54
3

Yes, it means "move the method to a separate class". If separating object construction and domain data initialization is desired, then this is fine and not "feature envy".

However, that case is rare. Most classes should initialize their invariants in the constructor, so that there can never be an invalid state at all. Having a class with guaranteed invariants makes reasoning about a system so much easier that it's worth expending some effort to make this possible.

-2

The author of the accepted answer from the link is a well-respected contributor on this stack and perhaps he will follow up with another answer or response here but I'm not completely sure about this part of his answer:

Here you have an object in two fundamentally different states: pre-init and post-init. Those states have different responsibilities, different methods that are allowed to be called, and different behavior. It's effectively two different classes.

That's not really my experience with the use of this idiom. Usually what I see with this is that an object is in an invalid state until the init() is called. That is, the object is effectively only partially constructed and doing anything other than calling init() on objects in this state will cause an error or has no effect.

I'm not sure how realistic an example it is, but I would say that main problem or 'smell' is having a public init() method. I've sometimes had reason to lazy load expensive resources only if and when they are used. A private init() method which does this when other methods are called isn't really a problem. I would prefer something like ensureInit() which I think makes it clearer that calling it multiple times is not going to create an issue. If the init() doesn't take any parameters, it becomes even more puzzling as to why it would be public. If the init() method is always called immediately after construction, then it's almost guaranteed that the init method is unnecessary and the result of some sort of misunderstanding around object construction.

If this init call happens somewhere else in the application, it implies that something outside of the class is managing the object lifecycle without providing anything that the class or object doesn't already have. This sort of 'outcapsulation' is a major smell in my opinion. If you need to conditionally allocate resources, based on knowledge outside of the scope of the class or object's responsibilities, I typically prefer using a factory of some sort.

A slightly more challenging situation is when the init takes some parameters. In this situation the object can't be fully constructed until some other information is provided. This again can often be solved by introducing some sort of factory so that the object can be fully constructed by its constructor.

There are many reasons to fully construct an object in its constructor. Simply put, the existence of objects that are in an invalid state presents an opportunity for operations on invalid objects to occur. Another primary one is that, in languages like Java, this is the main way you can enforce immutability. That is, you can only declare a member variable as final if you assign at the time of instantiation. Another big problem is that when a class depends on something else to finalize object construction, it can be really problematic if there's more than one place in the application where that might happen. It's very brittle and tends to result in tight coupling between classes. Your object is like an empty box after construction, and it needs something to fill it. You also need to make sure it's filled before it's used.

The first preference in this situation should not be to create a separate class, IMO. The preference would be to move whatever is in init() into the constructor and only create the object when it is needed, and its dependencies can be realized. As mentioned before, object factory approaches can be used to delegate that decision to other parts of the code. Bloch's Builder is a type of object factory useful for situations where construction logic is complex. It's a very powerful approach but it can be a little difficult to implement and maintain so I wouldn't use it if a simpler type of object factory can be used such as a static method. Prior to method references being supported in Java, using a static method was problematic and I think this is why Factory Method patterns are often used instead in older code bases and recommendations.

This last point seems to not be well understood. In pre-1.8 Java, if you wanted to pass an object creating method to another class, you really needed to have an object of a different class from the object that you were creating. For example, a simple form would be:

public class Demo {
    Demo(){
    }
}

public class DemoCreator
   public Demo create() {
        return new Demo();
   }
}

I'm guessing this is the type of physical separation that was recommended. However, once it became possible to pass method references in Java, this 'creator' class becomes largely extraneous. You can simply create a static factory method:

public class Demo {
    public static Demo create() {
        // logic which is difficult or impossible to 
        // manage in constructor here
        return new Demo();
    }

    Demo(){
       // init stuff here if you can
    }
}

Another thing that I think is poorly understood is that you can pass the constructor directly as if it is a static factory (and it is one, in a way) using Demo::new

3
  • Two separate classes are Builder or Factory and the original class without init().
    – Basilevs
    Commented Mar 18 at 17:00
  • @Basilevs You don't need a separate class to use static method for creating objects.
    – JimmyJames
    Commented Mar 18 at 17:48
  • @Basilevs You can also pass references to constructors in Java since 1.8.
    – JimmyJames
    Commented Mar 18 at 17:49

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