Thread: How much to do in a contructor

If construction fails, it is better to throw an exception, rather than to create an error when the user tries to use the socket. The programmer should not need to call isValid() after creation. Nor should he need to do so before send() and recv().

Otherwise, looks good.

You should NOT ever return early from a constructor [1]. If you can not obey that principle, then the code should not be part of the constructor [I'm sure someone will come up with a useful exception to this rule - but the general rule certainly is "do not return before end of construction" (aside from footnote 1)]. Throwing an exception, as King Mir says, would be a plausible alternative to splitting the construction.

In this particular case, I'd say your constructor should set member variables, and a "connect" or "initialize_connection" or whatever you want to call it, should deal with the socket create, bind, listen, etc. You can then also return an error message to say "didn't work", and not have to call "is_valid" from the code using the class.

Also, if bind, listen, create are all functions returning bool, why are you not checking the return value?

[1] Obviously, if there are situations where some argument is optional (e.g. you may pass a NULL or a char */std::string pointer, and if the value is NULL, then there is less work to do, and this work is done last, then you would be allowed to return early after sufficient work besides the copying of the string or whatever the operation would be to deal with the optional parameter).

--
Mats

I'm having a little trouble understanding how I would try/catch when creating a class variable, because the second I do it, the constructor is called.

This doesn't work. I get some errors about braces and this and that.

Code:

int main (void)
{
    server_socket ssck;

    try (ssck)
    {
        std::cout << "Socket Creation Successful!" << std::endl;
    }
    catch (err)
    {
        if (err == errcreate)
        {
            cout << "Error at create()" << endl;
        }
    }

    return 0;
}

I know I've deviated off of the original topic but we've kind of gone a new direction.

--Edit. Nevermind, I'm a bit rusty on the syntax. I got it though. sorry.

Thnaks again for your suggestions!

That looks almost right. Just move the "Socket Creation Successful!" message inside the try block as well since you don't want that to print when it failed.
I assume you are simply throwing an int?

Thank you phantomotap. I think I will get more descriptive later, but right now, I need to get the simplest version working so I can build off of it. I will most assuredly reference your post when I am ready to expand upon it.

Back to what I had originally, I apologize that I failed to mention the error I was receiving:

error: ‘ssck’ was not declared in this scope|

And yes iMalc, I am just throwing an int.

Code:

//Constructor
server_socket::server_socket(short int port)
{
    errcreate = 1;

    fport = port;
    fsock = 0;
    memset(&faddr, 0, sizeof(struct sockaddr_in));

    /* Create the socket */
    if (screate() == true){throw errcreate;}
}

I set it to true for testing.

errcreate is defined under public:

Edit: The main thing I am worried about is the scope of the class variable, which is only in the try block.

The try block doesn't need to be around object creation only but may be around a larger scope, perhaps even in a function higher up the call stack.

What you do in a constructor depends on your object oriented approach. If you want true RAII then you can init everything in the constructor and throw when/if they fail. One problem here is the example of if you allocate 2 objects on the heap successfully but the third one fails and you throw, you just leaked 2 objects on the heap. std::auto_ptr can help out with this as well as the boost smart pointers so it's not a huge issue if you handle it right.

The other approach is to initialize a class in a two-step process. The general rule here is don't perform any action in a constructor that could possibly fail. Another way of saying this is only perform operations that are guaranteed to succeed. Then you create an Init() function or Create() and it performs all the setup required for the object. Benefits to this approach is you know that an object will be created every time you call the constructor. Downside is it may not be very clear without some code spelunking on how to setup and create the object correctly. I would still call this a form of RAII because you are initializing prior to usage but via a two-step process.

Normally I use the latter approach because it is much simpler and does not require all the try catch blocks every time you create an object and there is no room for error when it comes to leaking memory since the constructor does not allocate any memory. Sometimes this approach has to be used in Win32 code because even though your C++ object exists your actual Win32 object may not which can cause problems.

Inevitably which approach you choose to use is up to you and I cannot say that one is more right or more wrong than the other. In an existing code line I would follow along the lines of what has already been done to maintain consistency.

Normally I use the latter approach because it is much simpler and does not require all the try catch blocks every time you create an object

I adhere very strictly to the former approach, and the only try-catch blocks I have in my project are at COM boundaries. I consider two-stage construction something to be avoided if possible. (And it usually can be avoided.)

If you use two-step process or more, then the object is is a non-usable state after the constructor, so it usually means extra work in making sure you can't use the object before the initialization process is complete or you might invoke undefined behavior.
An ideal constructor will make the object ready for use after it's finished.