Objects and classes when using IF statements

[billy_111]

Hi,

I am writing a program which tests a client and server. I am fairly new to C++ but have been ok in terms of coding..

However, i want to make my code look and work more efficiently. I currently have a series of IF statements checking for certain behaviors.

What i want to do it maybe have the IF statements in a separate class? And then call the functions how i need them?

This is my code, if you notice the IF's, you can see what i want to do, are there any examples on how i can do this?

sing namespace std;

bool DayTimeServer::run(TCPSocket *connSocket,
sockaddr_in *client_addr)
{
time_t curTime;
time(&curTime);
string cl_request;
ofstream LOGFile("daytime.log", ios::app); //name of txt file

string client_ip, timeStr, command_str, msg_str, msg_sz, msg, timeStr1;
int client_port, size;
int bytesSent, bytesRecv = SOCKET_ERROR;
bool status = true;

client_ip = inet_ntoa(client_addr->sin_addr);
client_port = ntohs(client_addr->sin_port);

cout <<endl<< "Client Connected; IP addr: "<<client_ip<<" at Port: "<<client_port<<endl; // echo out the client IP address and Port

//Read the command first.
command_str = receive(connSocket, 3);
string clienttime;

if (command_str == "ECH" || command_str == "REV")
{
msg_sz = receive(connSocket, 4); //message size
size = atoi(msg_sz.c_str()); //convert to string
msg = receive(connSocket, size); //actual message
cl_request = command_str+msg_sz+msg;
}
else if (command_str == "TIM")
{
clienttime = receive(connSocket, 10);
msg_sz = receive(connSocket, 4); //message size
size = atoi(msg_sz.c_str()); //convert to string
msg = receive(connSocket, size); //actual message
cl_request = command_str+msg_sz+msg;
}

timeStr1 = toString (curTime);
string response; //declare the response

if (command_str.size() > 0)
{
if (command_str == "ECH")
{
string input = command_str+msg_sz+msg;
if(input == "ECH0012hello world")
{
response = "ERRORFORMAT TYPE";
}
else
{
response = "ECHRP"+msg_sz+msg;
}
}
else if (command_str == "REV")
{
std::reverse(msg.begin(),msg.end()); //reverse message first
response = "REVRP"+msg_sz+msg;
}
else if (command_str == "TIM")
{
response = "TIMRP"+clienttime+timeStr1+msg_sz+msg;
}
else if (command_str == "SYN")
{
string timeStr1 = toString (curTime);
response = "SYNRP"+timeStr1;
}
else if (command_str == "UNK")
{
response = "ERRORUNKNOWN TYPE";
}
send(connSocket,response);

cout<<"Client's request: " <<command_str<<cl_request<<endl; //Notification
cout<<"Responded with: "<<response<<" "<<timeStr<<endl;
}

If you notice the If's, i want them to be functions that just need to be called?

Hope this can be done.

Regards

[billy_111]

Any ideas how i can do this?

[Amleto]

first step is ... code tags. so we can actually read your code

[VictorN]

... second step - explain what TCPSocket is

[billy_111]

Well basically i am writing a Windows Sockets program that implements an echo server. The server listens on a particular port which i physically type in on the command line. It then waits for clients to connect and send requests as specified by protocols in the code below.

Now this is just a test piece, and i want to experiment on this first and then i will be working on live projects. But i need to understand how i can make this code better and instead of just having a set of IF statements i can call the functions. I have done this using C# before but have no idea how it would work in C++.

Anyway, this is the code again:-

#include "DayTimeServer.h"
#include <iostream>
#include <string>
#include <sstream>
#include <time.h>
#include <iomanip>
#include <algorithm> 
#include <fstream> //need this for keeping log

using namespace std;

bool DayTimeServer::run(TCPSocket *connSocket, sockaddr_in *client_addr) 
{
	time_t	curTime;
	time(&curTime);
	string cl_request;

	string	client_ip, timeStr, command_str, msg_str, msg_sz, msg, timeStr1;
	int		client_port, size;
	int		bytesSent, bytesRecv = SOCKET_ERROR;
	bool	status = true;

	client_ip   = inet_ntoa(client_addr->sin_addr);
	client_port = ntohs(client_addr->sin_port);

	cout <<endl<< "Client Connected; IP addr: "<<client_ip<<" at Port: "<<client_port<<endl; // echo out the client IP address and Port
	
	//Read the command first.
	command_str = receive(connSocket, 3);
	string clienttime;

	if (command_str == "ECH" || command_str ==  "REV")	
	{
		msg_sz = receive(connSocket, 4); //message size
		size = atoi(msg_sz.c_str());    //convert to string
		msg = receive(connSocket, size); //actual message
		cl_request = command_str+msg_sz+msg;
	}
	else if (command_str == "TIM")
	{
		clienttime = receive(connSocket, 10);
		msg_sz = receive(connSocket, 4); //message size
		size = atoi(msg_sz.c_str());    //convert to string
		msg = receive(connSocket, size); //actual message
		cl_request = command_str+msg_sz+msg;
	}
	else if (command_str == "SYN")
	{
		cl_request = command_str;
	}
	else if (command_str == "UNK")
	{
		cl_request = command_str;
	}

	timeStr1 = toString (curTime);
	string response; //declare the response 

	if (command_str.size() > 0)
	{
		if (command_str == "ECH")									
		{
			string input = command_str+msg_sz+msg;
			if(input == "ECH0012hello world")
			{
				response = "ERRORFORMAT TYPE";
			}
			else
			{
			response = "ECHRP"+msg_sz+msg;							
			}
		}
		else if (command_str ==  "REV")								
		{
			std::reverse(msg.begin(),msg.end()); //reverse message first
			response = "REVRP"+msg_sz+msg;
		} 
		else if (command_str ==  "TIM")								
		{
			response = "TIMRP"+clienttime+timeStr1+msg_sz+msg;
		} 
		else if (command_str ==  "SYN")								
		{
			string timeStr1 = toString (curTime);
			response = "SYNRP"+timeStr1;
		}  
		else if (command_str ==  "UNK")	
		{
			response = "ERRORUNKNOWN TYPE";
		}
		send(connSocket,response);
		
		cout<<command_str<<endl;
		cout<<"Client's request: " <<cl_request<<endl;	 	//Notification 
		cout<<"Responded with: "<<response<<" "<<timeStr<<endl;
	}
	return status;
}

string toString (unsigned int val, unsigned short len)
{
	stringstream stream;
	stream <<setfill('0')<<setw(len)<< val ;

	return stream.str();
}

TCPSocket is declared in a separate .cpp file.

So is there a way where i can use a much cleaner way of writing the code i have written so far?

Regards

[VictorN]

What and how does this line suppose to do:

	//Read the command first.
	command_str = receive(connSocket, 3);

[billy_111]

It receives the size of the command message (3) is 3 bytes.

Is there something i can follow so i can use an Object Oriented approach to hide most of the data?

[Paul McKenzie]

But i need to understand how i can make this code better and instead of just having a set of IF statements i can call the functions. I have done this using C# before but have no idea how it would work in C++.

First, you need to simplify the first set of if() statements.

        cl_request = command_str;
        bool bTimFound = false;
	if (command_str == "ECH" || 
            command_str == "REV" || 
            (bTimFound = (command_str == "TIM" )))
	{
	        if ( bTimFound )
  		    clienttime = receive(connSocket, 10);
		msg_sz = receive(connSocket, 4); //message size
		size = atoi(msg_sz.c_str());    //convert to string
		msg = receive(connSocket, size); //actual message
		cl_request = command_str+msg_sz+msg;
       }

It doesn't "OO" the code, but it reduces some redundancy.

So is there a way where i can use a much cleaner way of writing the code i have written so far?

IMO, the code is not to the point of having to "clean it up", with the exception of making the if() statements more organized.

Now, if you had more places in the code where you are consistently checking for "REV", "TIM", etc. then that looks like a time to start introducing a "Command" class, and derive classes from this base class that represent each command. Then you introduce virtual functions that perform whatever operation you are trying to perform, for example GetCommandStr(), GetResponse(), etc. overriding these functions in those derived classes.

Regards,

Paul McKenzie

[billy_111]

Hey,

Thanks alot Paul.

That makes sense. So lets say i wanted to derive a Command class and wanted to call a function such as GetCommandStr(), how can i do this?

I have written classes before but not in regards to socket programming. I started from the basics of creating classes and calling them elsewhere..

For example, see the statement below:-

		else if (command_str ==  "REV")								
		{
			std::reverse(msg.begin(),msg.end()); //reverse message first
			response = "REVRP"+msg_sz+msg;
		}

This simply uses <algorithm> to reverse the message sent by the client. So taking into account the concept of classes and objects, would i be able to say something like:-

		else if (command_str ==  "REV")								
		{
			//CALL FUNCTION RESULT HERE...
		}

I hope you see where i am going with this..

Would i be able to do this?

Thanks again

[Paul McKenzie]

Hey,

Thanks alot Paul.

That makes sense. So lets say i wanted to derive a Command class and wanted to call a function such as GetCommandStr(), how can i do this?

Create a base class, the base class has a virtual function GetCommandStr(), and then implement that function in the derived class.

I have written classes before but not in regards to socket programming. I started from the basics of creating classes and calling them elsewhere.

#include <string>

class CommandObject
{
    public:
      CommandObject(const std::string& cmdString="") :
              command_str(cmdString) { }
       ~CommandObject() { }
       virtual void ComputeCLRequest) {}
       virtual bool CreateAndSendResponse() { return ""; }
       
   private:
       std::string command_str;  

  protected:
       std::string GetOrigCommandStr() const { return command_str; }  // just in case you need it for derived classes
};

class CommandREV : public CommandObject
{
   //...
   public: 
      CommandObject(const std::string& cmdString="") : CommandObject(cmdString) { }

       // Implement these functions
       void ComputeCLRequest) 
       { /* your implementation */ }
       bool CreateAndSendResponse() { /* your implementation */ }
   //...
};

class CommandECH : public CommandObject
{
   //...
};

class CommandTIM : public CommandObject
{
   //...
   std::string client_time;
};

CommandObject* CreateCmdObject(const std::string& cmd)
{
   if ( cmd == "REV" )
      return new CommandREV(cmd);
   else
   if ( cmd == "ECH" )
       return new CommandECH(cmd);
   else
   if ( cmd == "TIM" )
       return new CommandTIM(cmd);
//... etc...
   return 0; // no object created
}

Then the driver code would look like this:

//...
command_str = receive(connSocket, 3);
bool status = false;

CommandObject *pObject =CreateCmdObject(command_str);
if ( pObject )
{
     pObject->ComputeCLRequest();
     status = pObject->CreateAndSendResponse();
}
delete pObject;
return status;

This is just a skeleton, but you have to fill in any private variables, and implement the functions.

Regards,

Paul McKenzie

[billy_111]

Thank you very much Paul.

I really appreciate your help. My boss recommended the same as your suggestions so i thought id see how its done.

Anyway, i will let you know how i get on..

Regards
Billy