TCP socket server for UNIX

I'm very new to C++, but I really want to write good code and increase my development skill, so I ask you for some review.

Scheme of socket server

This is the scheme of how my socket server works. It creates a socket, binds it, sets to listen state, creates a new thread (in order not to block the execution of the program), which accepts new connections, creates new threads for each client, reads data from them and dispatches data to observers (server is a child of Observable class, which allows to dispatch events to Observers.

```
#include
#include
#include
#include
#include
#include
#include
#include "Observer.h"

using namespace std;

class SocketServer : public Observable
{
private:
string serverIp;
int serverPort;
int masterSocket;

void SocketCreate ( );
void SocketBind ( );
void SocketListen ( );
static void SocketAccept ( void );
static void SocketRead ( void );

void RequestDispatch ( int , string );

public:
SocketServer ( string , int );

static void Send ( int , string );
static void Log ( int , string );
};

struct ClientRequest
{
int socketFD;
string request;
};

struct ServerAndSocket
{
SocketServer* socketServer;
int socketFD;
};

SocketServer::SocketServer ( string serverIp , int serverPort )
{
this->serverIp = serverIp;
this->serverPort = serverPort;

this->SocketCreate();
this->SocketBind();
this->SocketListen();

pthread_create ( new pthread_t() , NULL , SocketServer::Socket

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include "Observer.h"

using namespace std;

This is fine for trivial demo code and such, but more often a mistake for anything larger or more serious.

class SocketServer : public Observable {

Personally, I think I'd prefer this as something like:

namespace Socket {
    class Server {
        // ...
    };
}

void                            SocketCreate();
    void                            SocketBind();
    void                            SocketListen();
    static void*                    SocketAccept(void*);
    static void*                    SocketRead(void*);

...then these would become just : Create, Bind, Listen, Accept, and Read (but the full names would be, for example, Socket::Server::Create, remaining quite self-explanatory).

SocketServer::SocketServer(string serverIp, int serverPort)
{
    this->serverIp = serverIp;
    this->serverPort = serverPort;

A member initializer list is generally preferable:

Server::Server(string serverIp, int serverPort) : serverIp(ServerIp), serverPort(serverPort) {

this->SocketCreate();
    this->SocketBind();
    this->SocketListen();

this-> is ugly noise. Avoid it.

pthread_create(new pthread_t(), NULL, SocketServer::SocketAccept, this);

If at all possible, you almost certainly want to use C++11's built-in thread support instead of using pthreads directly. The result is not only much more portable, but nearly always quite a bit cleaner as well.

void    SocketServer::SocketCreate()
{
    this->masterSocket = socket(AF_INET, SOCK_STREAM, 0);
    if (this->masterSocket < 0)

One more time: this-> is ugly noise that should be avoided whenever possible (and at least in C++, that's nearly always).

{
        perror("socket");
        _exit(0);
    }

In my opinion, this is a terrible idea. Code at this level shouldn't set policy about how errors are handled this way. It should (probably) retrieve the error message string using strerror, then throw an exception.

else
    {
        int opt = 1;
        setsockopt(this->masterSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof (opt));
        cout masterSocket << "\n";

Again, using cout for something on the order of logging seems a rather poor idea.

if (bind(this->masterSocket, (sockaddr*) &serverAddress, sizeof (serverAddress)) serverIp serverPort << " address\n";
    };

I'll note what a poor idea this is one last time, and leave it at that.

char                        input[256];

This size should probably be set with a named constant instead of a magic number.

memset((void*) &input, '\0', sizeof (input));

This seems to be pointless.You gain nothing from setting this memory to 0's immediately before read puts its data into the

inputLength = read(socketFD, (void*) &input, 255);

This should almost certainly use something like sizeof(input)-1 instead of 255.

StackExchange Code Review Q#14650, answer score: 4