Is it a good idea to split a constructor in multiple functions?

Question

Here is the work flow of a class of my program:

Server class instanciation -> Creating socket -> Binding socket to addr:port -> Listening -> Handling clients

Should I put all that stuff in the constructor or in separated private functions ?

Answer 1

As a rule, you should minimise the amount of business logic in constructors... So, depending on what your class's function/responsibility is, you may choose to include socket creation and binging the socket to an address in the constructor, but most likely listening and handling should be done in separate methods.

Answer 2

The constructor should do only one thing and do it well: construct the object (here an instance of Server).

No, don't do all this in the constructor ! Here is why

If you add more things such as creating a socket, and binding it to a port address, you will face multiple problems. For example:

• what if the port binding went wrong: should the constructor fail ? Or should it return the instance, but in an error state that could allow later recovery ?
• what if there could be different alternatives to do the same thing, for example bind the server to an IPv4 port or bind the server to an IPv6 address ?

Furthermore, if the constructor initiates operations that do not end, such as listening and handling client requests, all your code will execute on an unfinished object (i.e. before the constructor completes).

Finally, some languages will not allow to effectively derive a new ServerXXX type, and override some of its methods. In C++ for example, your Server constructor would run on the Server base object of ServerXXX and will not be aware of the overridden ServerXXX methods. Only once the Server constructor would be finished would it hand over to the ServerXXX constructor

How to do it better ?

There are several alternatives. Not knowing all the details, I could imagine to use a (GoF) builder pattern:

• An abstract builder would define the general interface for buidling your server, by building the different parts needed.
• A concrete builder would implement this interface for a given Server class, and provide the glue to assemble the parts.
• A "client" would create the appropriate concrete builder and inject it into the director
• A director would invoke the methods of the contrete buider in the appropriate order
• The client would invoke the concrete builder to get the final ready to use Server.

This kind of approach is not ideally suited to perform running business, i.e. listening and handling request. This is definitibely not part of the construction process. Furthermore, this could need multithreading, one thread doing the listening, and several other threads handling the requests for the connected clients.

For this part, I'd suggest to go for a template method pattern that implements the event loop for the server. The main code would then be something like:

ServerBuilder builder; 
ServerDirector director(builder); 
director.construct();                  // creates the server and its components
Server server = director.get_result(); // server ready to use 
server.mainloop();                     

Answer 3

I prefer constructors that return working instances of the given class. (Real-world analogy: if you order a car, you expect it to be functional in the sense that you can drive away with it, and that's what I expect from a constructor).

Of course, the definition of "working" is application-specific. So the question is, what do you expect from a Server?

• Class instanciation: You surely need to do that, otherwise there's nothing you could call a Server.
• Creating socket, Binding socket to addr:port, Listening: Is it a Server if it doesn't listen on any port?
• Handling clients: Is it a Server if it doesn't handle client requests?

Typically, I'd answer all these questions with a YES, and have the constructor do all these things (not creating a 100-lines long constructor, but breaking the actions down into individual methods, of course). But after the Server has been constructed, it serves.

Maybe your answers are different, or maybe Server is meant to be an abstract base class - then the constructor should be different, of course.

Answer 4

If your constructor is large enough that you're thinking hard about breaking it into several pieces, I'd tend to think about whether that might not indicate that your class (and its area of responsibility) is larger than it should be, so you might want to break the class into multiple pieces instead.

In the specific case of a socket server, I think there are really two (almost) completely separate parts. One just listens for incoming connections. It's pretty generic--almost any socket server does pretty much the same things: create a socket, bind it to an address, listen for incoming connections. Once that accepts a connection, it hands it off to some other class that has all the business logic for handling interactions with the client.

That second class is a session handler that just receives an open socket that's already been accepted, and communicates with the client via the socket. Harder to say exactly what happens here, because most of it depends on that interaction between the client and server.

When I've written code like this, neither class has had a constructor so large or complex that I saw much (any, really) reason to break it up into pieces. The listener basically does socket(), bind(), and listen(). Something like a dozen lines of code--and three fourths of that is error handling and such (though if you need to handle something like SSL/TLS, the complexity does grow somewhat).

Answer 5

Others answers cover most well. One thing not mentioned is how IDisposable(.NET) and AutoClosable and Closable(Java) interfaces work.

It is good practice in .NET to use the using statement and try-with-resources where possible.

If we put the logic in the constructor this would not be possible. So that is one more "con" for the pros and cons list.

Answer 6

Assuming you have a class which, among other things, handles the initial binding of the port and setting up event handling as a must (aka absolutely must run before any other operation can be performed), one way you could do it is to create a public method init that must be called before any other. ...But that's ugly. More to the point, it requires that the caller take considerations that should be obscured by the overall class itself.

A better solution would be to lazily initialize the socket and event handling setup. Which is to say, you have an instance of Server with a private boolean member isInitialized initially set to false. Before any method is called, you perform this check and perform the necessary call to private method init, afterwards setting isInitialized to true. If you have few public methods, this is ideal and dirties your actual implementation very little. If you have many public methods, you should consider creating a second class which deals with the nitty gritty of the connection, which you use to perform the low-level actions, including establishing when the connection is formed (all of which would be obscured from the caller to Server).

While it's true that you're not likely going to create a Server instance without using it (so the secondary benefit of lazy loading doesn't really apply), it still provides you a means to remove potentially explosive code inside your constructor. The error, too, would be more thrown in a more intuitive spot, in a called method init inside your send method, which is likely a place which will be covered by try catch blocks anyway.

Answer 7

I certainly prefer breaking apart big constructors, with a caveat:

I would only use static, preferably pure, methods for the job. This alleviates the need to worry about calling instance methods on an inconsistent instance.

For example, I might have something like:

class MyClass {
private:
    Socket socket;
public:
    MyClass(Address address, Port port) {
        this->socket = MyClass::createSocket();
        MyClass::bindSocket(this->socket, address, port);
        // etc
    }

    Socket createSocket() const {
        return Socket(); // hypothetical. Add socket construction logic here
    }

    void bindSocket(Socket socket, Address address, Port port) {
        socket.bind(address, port); //hypothetical
    }
};