Simple Socket class hierarchy

Question

One of the things I wanted to do was make it impossible to forget to initialise or forget to cleanup WSA. Often the network examples will return as soon as an error occurs (say an socket didn't bind) this means you have to remember to add WSACleanup() before each return and I don't like that.

I also wanted to make it impossible to forget to close opened sockets (I believe this is the RAII idiom?)

A quick rundown of the class structure:

• Socket (abstract base class)
• ListeningSocket (binds to a port and listens)
• AcceptingSocket (accepts connections)
• TransmittingSocket (connects to a remote machine to send data).

Socket.h:

#ifndef SOCKET_H
#define SOCKET_H

#include <iostream>
#include <WinSock2.h>

class Socket
{
public:
    virtual ~Socket() = 0;
    static bool winsockInitialized();
    bool isValid();
    SOCKET getSocket();

protected:
    Socket(SOCKET socket);

    SOCKET socket_;
    sockaddr_in socketAddress_;

private:
    Socket();

    class WsaData
    {
    public:
        WsaData()
        {
            std::cout << "Initializing WSA." << std::endl;
            initSuccess_ = WSAStartup(MAKEWORD(2,2), &wsaData_);

            if(initSuccess_ != 0)
            {
                std::cout << "Error: " << WSAGetLastError() << " occured." << std::endl; 
                WSACleanup();
            }
        }


        ~WsaData()
        {
            std::cout << "Cleaning up WSA." << std::endl;
            WSACleanup();
        }


        bool isInitialized() 
        {
            return !initSuccess_;
        }

    private:
        WSADATA wsaData_;
        int initSuccess_;
    };

    static WsaData wsaData_;
};

#endif

Socket.cpp:

#include "Socket.h"

#include <iostream>

Socket::WsaData Socket::wsaData_;

// Pure virtual destructor
Socket::~Socket()
{
    if(socket_ != INVALID_SOCKET)
    {
        closesocket(socket_);
    }
}


// Accessor for WSA initialization status
bool Socket::winsockInitialized()
{
    return wsaData_.isInitialized();
}


// Implicit conversion from Socket to underlying SOCKET
bool Socket::isValid()
{
    return socket_ != INVALID_SOCKET;
}


// Accessort for SOCKET member
SOCKET Socket::getSocket()
{
     return socket_;
}


// Custom constructor
Socket::Socket(SOCKET socket) : socket_(socket)
{
}


// Default constructor (hidden)
Socket::Socket()
{
}

ListeningSocket.h:

#ifndef LISTENING_SOCKET_H
#define LISTENING_SOCKET_H

#include "Socket.h"

class ListeningSocket : public Socket
{
public:
    ListeningSocket(int port);
    ~ListeningSocket();

    bool bindAndListen();

private:
    ListeningSocket();

    const int port_;
};

#endif

ListeningSocket.cpp:

#include "ListeningSocket.h"

// Custom Constructor
ListeningSocket::ListeningSocket(int port): port_(port), Socket(socket(AF_INET, SOCK_STREAM, 0))
{
    socketAddress_.sin_port = htons(port);
    socketAddress_.sin_addr.s_addr = INADDR_ANY;
    socketAddress_.sin_family = AF_INET;
}


// Destructor
ListeningSocket::~ListeningSocket()
{
    std::cout << "Listening socket closing." << std::endl;
}


// Binds the socket to and listens on the specified port
bool ListeningSocket::bindAndListen()
{
    int bindError = bind(socket_, (sockaddr*)&socketAddress_, sizeof(socketAddress_));
    if(bindError == SOCKET_ERROR)
    {
        std::cout << "Error: " << WSAGetLastError() << " while binding socket to port " << port_ << "." << std::endl;
        return false;
    }

    std::cout << "Socket bound on port " << port_ << "." << std::endl;

    int listeningError = listen(socket_, SOMAXCONN);
    if(listeningError == SOCKET_ERROR)
    {
        std::cout << "Error: " << WSAGetLastError() << " while trying to listen on port " << port_ << "." << std::endl; 
        return false;
    }

    std::cout << "Socket listening on port " << port_ << "." << std::endl;

    return true;
}


// Default Constructor (hidden)
ListeningSocket::ListeningSocket(): port_(0), Socket(socket(AF_INET, SOCK_STREAM, 0))
{
}

AcceptingSocket.h:

#ifndef ACCEPTING_SOCKET_H
#define ACCEPTING_SOCKET_H

#include "Socket.h"

class AcceptingSocket: public Socket
{
public:
    AcceptingSocket(Socket& socket);
    ~AcceptingSocket();

private:
    AcceptingSocket();
};

#endif

AcceptingSocket.cpp:

#include "AcceptingSocket.h"

// Custom constructor
AcceptingSocket::AcceptingSocket(Socket& socket) : Socket(accept(socket.getSocket(), NULL, NULL))
{
}


// Destructor
AcceptingSocket::~AcceptingSocket()
{
    std::cout << "Accepting socket closing." << std::endl;
}


// Default constructor (hidden)
AcceptingSocket::AcceptingSocket() : Socket(socket(AF_INET, SOCK_STREAM, 0))
{
}

TransmittingSocket.h:

#ifndef TRANSMITTING_SOCKET_H
#define TRANSMITTING_SOCKET_H

#include "Socket.h"

#include <string>

class TransmittingSocket: public Socket
{
public:
    TransmittingSocket(std::string ip, int port);
    ~TransmittingSocket();

    bool connectTo();

private:
    TransmittingSocket();

    const std::string ip_;
    const int port_;
};

#endif

TransmittingSocket.cpp:

#include "TransmittingSocket.h"

#include <iostream>

// Custom constructor
TransmittingSocket::TransmittingSocket(std::string ip, int port): ip_(ip), port_(port),
    Socket(socket(AF_INET, SOCK_STREAM, 0))
{
    socketAddress_.sin_port = htons(port_);
    socketAddress_.sin_addr.s_addr = inet_addr(ip_.c_str());
    socketAddress_.sin_family = AF_INET;
}


// Destructor
TransmittingSocket::~TransmittingSocket()
{
    std::cout << "Transmitting socket closing." << std::endl;
}


// Connects the socket to specified ip/port
bool TransmittingSocket::connectTo()
{
    int connectionError = connect(socket_, (sockaddr*)&socketAddress_, sizeof(socketAddress_));

    if(connectionError == INVALID_SOCKET)
        return false;
    else
        return true;
}


// Default constructor (hidden)
TransmittingSocket::TransmittingSocket(): ip_(""), port_(0), Socket(socket(AF_INET, SOCK_STREAM, 0))
{
}

And finally, here is a really simple client/server example using these classes:

Client:

#include <iostream>
#include <vector>

#include "TransmittingSocket.h"

int main(int argc, char* argv[])
{
    if(!Socket::winsockInitialized())
        return -1;

    TransmittingSocket transmitSocket("127.0.0.1", 100);

    if(!transmitSocket.isValid())
        return -1;

    if(!transmitSocket.connectTo())
        return -1;

    std::string text = "Some sentence with whitespace in it.";
    int bytesSent = send(transmitSocket.getSocket(), &text[0], sizeof(char)*text.size(), 0);
    std::cout << "Sent " << bytesSent << " bytes." << std::endl;

    system("pause");
    return 0;
}

Server:

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <WinSock2.h>

#include "ListeningSocket.h"
#include "AcceptingSocket.h"

int main(int argc, char* argv[])
{
    if(!Socket::winsockInitialized())
        return -1;

    ListeningSocket listeningSocket(100);

    if(!listeningSocket.bindAndListen())
        return -1;

    AcceptingSocket clientSocket(listeningSocket);

    if(!clientSocket.isValid())
        return -1;

    std::vector<char> buffer = std::vector<char>(1024, 0);
    int recvBytes = recv(clientSocket.getSocket(), &buffer[0], sizeof(char)*buffer.size(), 0);
    std::string text = std::string(&buffer[0], &buffer[recvBytes]);
    std::cout << text << std::endl;
    buffer.clear();

    std::ofstream outfile("C:\\Users\\Borgleader\\Documents\\Visual Studio 2010\\Projects\\Hex\\Debug\\text.txt",'w');
    if (outfile.is_open())
    {
        outfile << text;
        outfile.close();
    }
    else
    {
        std::cout << "Error opening file";
    }

    std::cout << "File was sucessfully received.\n";

    system("pause");
    return 0;
}

Answer 1

Without looking at the details, it seems that you have too many classes. A base class for all sockets that takes care of freeing the resources is fine. A listen socket that waits for incoming connections is fine, too.

But I don't get the need for AcceptingSocket and TransmittingSocket, because once you've got a connection it doesn't really matter where it came from, you can use it to read and/or write data.

So my suggestion is to use one ConnectionSocket that can be made to either accept an incoming connection from a listen socket, or actively open a connection to somewhere, depending on which constructor is used.