Any problem with doing the main work of a class in its constructor?

I have always felt that in general the 开发者_开发知识库main work of a class should be done in its instance methods, while the constructor should only get the instance into a usable inital state.

But I find that in practice there are situations where it seems to make more sense to put essentially all the actual work into the constructor.

One example: I need to retrieve some DBMS-specific information from the database. The most natural way to me seemed to have a class DBMSSpecInfo, with a constructor:

public DBMSSpecInfo(java.sql.Connection conn) throws SQLException{
  // ... retrieve info from DBMS
}

/** @returns max size of table in kiB */
public int getMaxTableSize() {//...}

/** @returns max size of index in kiB */
public int getMaxIndexSize() {//...}

/** @returns name of default schema */
public String getDefaultSchema() {//...}

You would construct the class once, the constructor would fetch all data, then you could use various getters to retrieve the info you need.

Of course I could put the method somewhere else, and only use DBMSSpecInfo for the return value (essentially using DBMSSpecInfo only as a value holder), but it feels ugly to create a class just for returning values from a single function.

So what do you think? Are there problems with performing the main work in the constructor? Is it "un-idiomatic" in Java? Or is it an acceptable (though possibly uncommon) practice?

The main practical problem is unit-testing - you won't be able to instantiate the object without doing actual work. (Or you'd have to mock all the classes that participate in this work).

Related talk: OO Design for testability. It gives examples of why doing work in constructors is bad for unit-testing.

I would prefer separating the creation code from the class itself in such cases. It could be put into a static factory method, or a separate factory class (which can also be a public static inner class). The choice depends on the complexity of the code and the design context (which we don't know in this case).

This would also allow you to do optimizations, like caching and reusing the class instance(s).

I'm big on pragmatism. If it works, do it! But in the name of purity and goodness, I'd like to make a design suggestion:

This class muddles up the data content with the mechanism for retrieving it. The object you end up using elsewhere is interesting only for the data it contains. So the "clean" thing to do would be to have a different class for digging out the information and then creating instances of this properties object.

That other class could have a longer lifetime, as you'd typically be calling a method to do the work, not the constructor. The constructor of DBMSSpecInfo might end up assigning a bunch of properties but not doing a lot of error-capable DB access work.

In your example I would make a static method GetDBMSSpecInfo(java.sql.Connection conn) that will return an instance of DBMSSpecInfo object or null if something goes wrong (in case you don't want to throw exceptions).

The DBMSSpecInfo object for me should not contain nothing more than get properties: MaxIndexSize, MaxTableSize, DefaultSchema, etc.

And I would make the constructor of this object private so that instances can only be created from the static method.

I don't think it is a good idea to do the main work in a constructor, since it doesn't have a return value. So it makes error processing more complicated IMO, since it forces you to use exceptions.

A disadvantage of doing the work in the constructor is that constructors can not be overridden (nor should they delegate to overridable methods).

Another is that a constructor is all-or-nothing. If the object contains data whose initializations exhibit indepedent failures, you deprive yourself of the capability to use what data could be procured successfully. Similarly, that you have to initialize the entire object, even if you just need part of it, might adversely affect performance.

On the other hand, doing it in the constructor allows initialization state (here: the connection to the database) to be shared, and released earlier.

As always, different approaches are preferable in different circumstances.

Doing all the work in the constructor can lead to "overload hell". You keep wanting to add more features and instead of just adding a new method, like you would in normal Object-Oriented development, you find yourself adding more and more overloaded constructors. Eventually, the constructors can grow so many overloads and parameters that it becomes unwieldy.

Just be careful that the object is not cloned/deserialised. Instances created this way do not use the constructor.

In my opinion the constructor should be lightweighted and should not throw exceptions. I'd implement some kind of Load() method to retreive data from the database, or implement lazy loading.

No problem. JDK has a lot of classes that does network IO in constructors.