irc bot not working

[sveatlo]

Hi everybody!
I'm learning network programming and I've decided to program a simple c++ irc bot.
I'm able to connect to server and send commands. But when I try to receive and show message from server I first get a few lines, like "Please, wait, while we process your connection" or "Found yout hostname". And then I start to get a ton of informations about the server.
Here is the whole class:

header file:

#pragma once
#include <WinSock2.h>
#include <WS2tcpip.h>
class bot
{
public:
	bot(char *_server, char *_port);
	~bot(void);
private:
	char *server;
	char *port;
	WSADATA wsadata;
	SOCKET sock;
	int status, result;
	struct addrinfo hints;
	struct addrinfo *res;
	
	bool connect_to_server();
	void connect_to_irc();
	void send_to_server(char *buffer);
	void read();
};

source file:

#include <iostream>
#include <string>
#include <WinSock2.h>
#include <WS2tcpip.h>
#include "bot.h"

using namespace std;
bot::bot(char *_server, char *_port) {
	server = _server;
	port = _port;

	if(connect_to_server())
		connect_to_irc();
		read();
}

bot::~bot(void) {
	closesocket(sock);
	freeaddrinfo(res);
    WSACleanup();
}
bool bot::connect_to_server(){

	result = WSAStartup(MAKEWORD(2, 2), &wsadata);
    if (result != 0) {
        printf("WSAStartup failed: %d\n", result);
        return false;
    }

	memset(&hints, 0, sizeof hints);
	hints.ai_family = AF_UNSPEC;
	hints.ai_socktype = SOCK_STREAM;
	hints.ai_flags = AI_PASSIVE; 

	if ((status = getaddrinfo(server, port, &hints, &res)) != 0) {
		cout << "getaddrinfo error: " << gai_strerror(status) << endl;
        return false;
	}
	if((sock = socket(res->ai_family, res->ai_socktype, res->ai_protocol)) == -1){
		cout << "socket error: " << errno;
        return false;
	}
	if(connect(sock, res->ai_addr, res->ai_addrlen) == -1){
		cout << "connection error: " << errno << endl;
        return false;
	}
	cout << "connection successful" << endl;
	return true;

}
void bot::connect_to_irc(){
	cout << "sending irc commands ..." << endl;
	send(sock, "NICK sveatlo_bot\r\n", strlen("NICK sveatlo_bot\r\n"), NULL);
	send(sock, "USER sveatlo_bot 0 0 sveatlo_bot\r\n", strlen("USER sveatlo_bot 0 0 sveatlo_bot\r\n"), NULL);
	send(sock, "JOIN #ircbot", strlen("JOIN #ircbot"), NULL);
}
void bot::read(){
	cout << "reading ..." << endl;
	char buffer[512] = {0};
	//wtf?!

	while(1){
		recv(sock, buffer, sizeof(buffer), NULL);
		string str(buffer);
		// string str ("PING :0123456789");
		string str1 = str;
		string str2 ("PING");
		char *msg;
		unsigned found = str1.find(str2);
		if (found!=std::string::npos)
			str1.replace(str1.find(str2), str2.length(), "PONG");
			msg = (char*)str1.c_str();
			cout << msg << endl;
			send(sock, msg, strlen(msg), NULL);
		cout << str << endl;
	}
}

Link to whole visual studio project: http://rapidshare.com/files/336797813/IRCbot.7z
Am I doing something wrong during the IRC connection? Or I didn't understand how networking work?

Thanks in advance

[3qX21hU5]

I am really really bad at network programming so I can't really help you out with any problems with this.

But I did want to point out one thing I noticed about your class just by reading your post.

You don't have a default constructor. ALWAYS use a default constructor. Normally if you don't declare any constructors the compiler will automatically generate a default constructor for you, but since you have a constructor defined bot(char *_server, char *_port); which it will no longer generate a default one automatically. This might cause you problems in the long run that can be painful to track down.

Also since you have pointers in your class you should also be manually declaring a copy constructor also.

Just some tips but unfortunately they most likely won't fix your problem, but wish ya the best of luck with the project.

[LB]

@Zereo I would like to know what problems could occur from not having a default constructor. There are millions of C++ classes that could not work at all with a default constructor.

[doug4]

+1 to @L B. There are many reasons to not have a default constructor, and this certainly could be one of them.

[3qX21hU5]

@Zereo I would like to know what problems could occur from not having a default constructor. There are millions of C++ classes that could not work at all with a default constructor.

1) Well for this example since he doesn't have a default constructor these two variables are undefined and could lead to undefined behavior

int status, result;

2) Every constructor for every class that uses his class type will need to explicitly initialize the class members for his class.

3) Classes that have members of bot type will not implicitly generate a default constructor for there class so they to would also have to explicitly define a default constructor and have that constructor explicitly initialize bot type's members

4) His class can not be used as a element in a dynamically allocated array. If the array is statically allocated it must define a explicit initializer for each element.

5) These might be me misunderstand some things but I believe he can only pass and return type bot to/from a function by only reference since the copy constructor requires a default constructor to make the temporary objects. Also I believe some containers like vectors will be out of the question also since I believe they use the copy constructor which uses the default constructor to make the elements. But like I said this might be incorrect or me misunderstand how the copy constructor works.

That is just a few.

There are a lot of issues/bugs that can arise and it also significantly reduces the classes usability to not have a default constructor.

But really I was just pointing out to him that if he defines any type of constructor the compiler will no longer generate a default one for him automatically.

+1 to @L B. There are many reasons to not have a default constructor, and this certainly could be one of them.

Could you please name one instance where you would not want a default constructor? I am just curious and I could be wrong but I don't see any reason why you would not have a default constructor either implicitly or explicitly.

Like I said I could be wrong but I see almost no use of not having a default constructor...

[LB]

Zereo wrote:

1) Well for this example since he doesn't have a default constructor these two variables are undefined and could lead to undefined behavior

int status, result;

Member variables are alwways initialized via their default constructor if the class constructor being invoked does not initialize them otherwise. In this case, int's default ctor initalizes them to 0.

Zereo wrote:

2) Also without a default constructor he will error out if he tries to do something like this bot myBot;.

That is the purpose of a default constructor - some classes need to have parameters passed to them to be constructed.

Zereo wrote:

3) Since he has no default constructor he can't use any containers with his class type since the copy constructor needs the default constructor to create the temporary object.

The default constructor is not related to the copy constructor, and even so I don't think he needs STL container support. If he did, he could add the required member functions.

Zereo wrote:

There are a lot of issues/bugs that can arise and it also significantly reduces the classes usability to not have a default constructor.

You are mislead.

Zereo wrote:

Could you please name one instance where you would not want a default constructor? I am just curious and I could be wrong but I don't see any reason why you would not have a default constructor either implicitly or explicitly.

Like I said I could be wrong but I see almost no use of not having a default constructor...

All references in C++ do not have a default constructor.
Automatic pointer classes generally have no default constructor because they need to know which pointer to manage.
Any class containing references cannot have a default constructor.
std::istream and std::ostream have no default constructor.
etc...

[3qX21hU5]

Member variables are alwways initialized via their default constructor if the class constructor being invoked does not initialize them otherwise. In this case, int's default ctor initalizes them to 0

umm... int doesn't have a default constructor it is a integral type.. .It would be initialized to 0 if it was in global scope, and undefined if it is in local scope. Also that brings another reason to the table like you said member variable of class types are always initialized via their default constructor if they are not explicitly initialized so that could lead to errors by users of the class.

That is the purpose of a default constructor - some classes need to have parameters passed to them to be constructed.

I don't think you get what I said... He doesn't have a default constructor... The reason he doesn't have one is because he declared another constructor bot(char *_server, char *_port); and never explicitly declared a default constructor bot(); so he doesn't have one. I realize some classes could use parameters passed to them to construct them by why not declare that AND a default ctor?

The default constructor is not related to the copy constructor, and even so I don't think he needs STL container support. If he did, he could add the required member functions.

??? Take vector<string> svec(5); for example. The compiler first initializes svec by using the default string constructor to create a temporary value. Then the copy constructor is used to copy the temporary into each element of svec. So how does the copy constructor not use the default constructor?

You are mislead.

I actually don't think I am. Yes there are rare instances where you don't need a default constructor but I can't see any situation where it would hurt the program to have one though there probably is some situations.

std::istream and std::ostream have no default constructor.

How would you be able to do this std::ofstream out; or std::istream in;? Would that be calling the default constructor?

Also I revised what I posted before to have more concrete examples.



But anyways I wasn't trying to get into a big debate about constructors I was just letting the OP know that he doesn't have a default constructor in the code he posted, and that might not be what he wanted.

[doug4]

I use classes all the time with reference members. As @LB said, they never have a default constructor. In this case, the OPer wants his bot constructor to connect to a server and port. That make perfect sense to me. The constructor can't do that if it has defaulted values for host and port. This is a GOOD reason to not have a default constructor.

1) Well for this example since he doesn't have a default constructor these two variables are undefined and could lead to undefined behavior

That's a problem with the defined constructor, not with the lack of a default constructor. If the default constructor only defaulted host and port (like the given constructor), it would have the same problem.

2) Every constructor for every class that uses his class type will need to explicitly initialize the class members for his class.

I really don't understand this argument. The non-default constructor will have to initialize just as many members as a default constructor. Having a default constructor doesn't magically cause members to become initialized.

3) Classes that have members of bot type will not implicitly generate a default constructor for there class so they to would also have to explicitly define a default constructor and have that constructor explicitly initialize bot type's members

Right. Since a bot will connect upon construction, it needs to be initialized. Containing objects will have to initialize the bots upon their construction. If that's the desired behavior, then that's perfectly fine. I think this make good sense in this situation.

4) His class can not be used as a element in a dynamically allocated array. If the array is statically allocated it must define a explicit initializer for each element.

This is a good point, and I almost brought it up in my original reply. If there needs to be an array of bots, or even a STL Container containing bots, then this might not be the correct implementation for the bot class. Buyer beware.

5) These might be me misunderstand some things but I believe he can only pass and return type bot to/from a function by only reference since the copy constructor requires a default constructor to make the temporary objects. Also I believe some containers like vectors will be out of the question also since I believe they use the copy constructor which uses the default constructor to make the elements. But like I said this might be incorrect or me misunderstand how the copy constructor works.

As said earlier (I think-I don't see it now), it's not a problem. The copy constructor handles all this for you. Actually, in this situation, I would make the copy constructor and assignment operators private and not define them so the objects cannot be copied. Destroying a copy would close the connection prematurely. All passing of these objects to functions should be through references.

Could you please name one instance where you would not want a default constructor? I am just curious and I could be wrong but I don't see any reason why you would not have a default constructor either implicitly or explicitly.

I write a lot of classes where the constructor preforms some action and the destructor cleans up afterwords. One of my favorite is to grab a mutex on construction and release it on destruction. The constructor HAS to take a reference to the mutex or it wouldn't know what mutext to grab or release.

I don't know a whole lot about irc communication, so I can't debug the problem. But if I were to write the OP's code, it would look a lot like it currently is. I like the general design. I would probably name the class "Connection". When Connection exists, it will have been connected. When Connection is destroyed, it will disconnect. In this case you absolutely need a 2-argument constructor and no default constructor.

And we all realize that adding a non-default constructor removes the system-created default constructor. And we think it can be a good idea in many situations--like this one.

[LB]

Zereo wrote:

umm... int doesn't have a default constructor it is a integral type.. .It would be initialized to 0 if it was in global scope, and undefined if it is in local scope.

Primitive types do have constructors, and on top of that they do have default constructors:

int x = int(5); //copy ctor
int y = 1 + int(); //default ctor 

http://ideone.com/5F1gDw

Zereo wrote:

Also that brings another reason to the table like you said member variable of class types are always initialized via their default constructor if they are not explicitly initialized so that could lead to errors by users of the class.

Users of the class are not responsible for initializing the class members, this is the responsibility of the class constructor.

struct ExampleClass
{
    int x;
    int y;
    ExampleClass(int z) : x(5)
    {
        y += z;
    }
};
//...
ExampleClass my_example_instance (1);

http://ideone.com/HKh30L

Zereo wrote:

I don't think you get what I said... He doesn't have a default constructor...

Sorry, I meant "That is the purpose of not having a default constructor." I mis-typed.

Zereo wrote:

I realize some classes could use parameters passed to them to construct them by why not declare that AND a default ctor?

Because in some cases, as with the ones I mentioned, it is not possible nor does it make sense for a class to have a default constructor.

Zereo wrote:

??? Take vector<string> svec(5); for example. The compiler first initializes svec by using the default string constructor to create a temporary value. Then the copy constructor is used to copy the temporary into each element of svec. So how does the copy constructor not use the default constructor?

No, the elements are constructed in-place and the copy constructor is never called. However, it is perfectly possible to use std::vector with a class that has no default constructor. All you have to do is not using any functions of std::vector which require a default ctor, and the code will work.

struct NoDefCtor
{
    std::string s;
    NoDefCtor(const char *str) : s(str)
    {
    }
};
//...
std::vector<NoDefCtor> v;
v.push_back("Hello, ");
v.push_back("World!");
for(std::size_t i = 0; i < v.size(); ++i)
{
    std::cout << v[i].s << std::flush;
}

http://ideone.com/XvA2g0

Zereo wrote:

I can't see any situation where it would hurt the program to have one though there probably is some situations.

You seem to have been arguing that not having a default constructor is harmful to the program; this is where I claim you are mislead.

Zereo wrote:

How would you be able to do this std::ofstream out; or std::istream in;? Would that be calling the default constructor?

You cannot do that because it does not make sense. std::istream and std::ostream can only be instantiated from a std::streambuf instance. You cannot have a stream with no stream buffer. Besides, what would the default constructor do? Would the stream be a pointless stream? Why would you do this? It makes no sense.

Zereo wrote:

I wasn't trying to get into a big debate about constructors I was just letting the OP know that he doesn't have a default constructor in the code he posted, and that might not be what he wanted.

I am just trying to make sure that neither you nor the OP is confused about the implications of having or not having a default constructor.

[3qX21hU5]

Sigh I don't think you understood any of my points LB because you are using totally different examples for your arguements.

1)

Primitive types do have constructors, and on top of that they do have default constructors:

Like I said int's need to be explicitly initialized other wise they will be undefined behavior if they are in local scope. You are saying you don't need to explicitly initialize them because their constructor will do that for them which is false.

class Number
{
    public:
        // Implicit CTOR since we didn't define one
        // Non initialized member
        int notGood;
};

// isGood.notGood will be 0;
Number isGood;

int main()
{
    Number wontWork;
    cout << wontWork.notGood << endl;
    cout << isGood.notGood << endl;
    return 0;
}

Tell me what behavior you get? Is notGood 0?


2)

Users of the class are not responsible for initializing the class members, this is the responsibility of the class constructor.

I don't think you understood what I said. If he doesn't have a default a default constructor and the user uses his class type in a class of his own and assumes that it has a default constructor it will lead to a frustrating error. It also passes of the responsibility to the user of the class to initialize the members of class.

class Number
{
    public:
        Number(int num);

        int notGood;
};

class userClass
{
    userClass();
    
    // This will member not be initialized automatically
    Number stuff;
};

3)

No, the elements are constructed in-place and the copy constructor is never called. However, it is perfectly possible to use std::vector with a class that has no default constructor. All you have to do is not using any functions of std::vector which require a default ctor, and the code will work.

You are wrong with the example I gave you it uses the default constructor to make the temporary object and then uses the copy constructor to copy it into the vector.

Does this run?

class Number
{
    public:
        Number(int num) : notGood(0) {}

        int notGood;
};

int main()
{
    vector<Number> vec(5);

    return 0;
}

No it doesn't because it can't call the default constructor. Your example is different and you are right you can use vectors as long as you don't use some functions.


4)

You seem to have been arguing that not having a default constructor is harmful to the program; this is where I claim you are mislead.

For the average program yes it can be harmful to not have a default constructor. Though as you have mentioned there is some cases and I will agree with that now.

5)

I am just trying to make sure that neither you nor the OP is confused about the implications of having or not having a default constructor.

I am going off of what I have learned from Effective C++ and C++ Primer and most of the cases that I listed are coming right out of the book. So i'm going with their word on it =/.

[3qX21hU5]

@doug4

I use classes all the time with reference members. As @LB said, they never have a default constructor. In this case, the OPer wants his bot constructor to connect to a server and port. That make perfect sense to me. The constructor can't do that if it has defaulted values for host and port. This is a GOOD reason to not have a default constructor.

I agree it is good to have the server and port to create the object but why is that a reason to not have a default constructor along with a constructor for creating one with the server and port? You guys are making it seem like you can only have one... There is such things as overloading constructors...

Right. Since a bot will connect upon construction, it needs to be initialized. Containing objects will have to initialize the bots upon their construction. If that's the desired behavior, then that's perfectly fine. I think this make good sense in this situation.

I agree and wasn't really listing things that could be wrong specifically for the OP program was just listing things that could go wrong in general for any program.


Anyways lets stop hijacking this thread since it is not the lounge and we are being disrespectful to the OP. Sorry about that sveatlo.

[LB]

Zereo wrote:

Tell me what behavior you get? Is notGood 0?

Both outputs are 0. Did you even test this?
Proof: http://ideone.com/tITNKY

Zereo wrote:

If he doesn't have a default a default constructor and the user uses his class type in a class of his own and assumes that it has a default constructor it will lead to a frustrating error.

It is his fault for assuming that is has a default constructor, and it is a simple fix as well given that they should be able to understand the compiler error.

Zereo wrote:

You are wrong with the example I gave you it uses the default constructor to make the temporary object and then uses the copy constructor to copy it into the vector.

http://ideone.com/2N6lkb
http://ideone.com/uXXvlJ
Point: it does not require the copy ctor (I was originally wrong about this)

http://ideone.com/XvA2g0
Point: it does not require the default ctor (I was originally right about this)

Zereo wrote:

For the average program yes it can be harmful to not have a default constructor.

The only harm is making the assumption that a type has a default ctor. Not having a default ctor does not cause any harm.

Zereo wrote:

I am going off of what I have learned from Effective C++ and C++ Primer and most of the cases that I listed are coming right out of the book. So i'm going with their word on it =/.

Which C++ Primer? There is a good one and a bad one. Also, could you provide page numbers and copy the exact argument/examples they give? EDIT: Oh, and which edition of Effective C++?

[3qX21hU5]

Both outputs are 0. Did you even test this?
Proof: http://ideone.com/tITNKY

Umm did you? Because both codeblocks and VS2010 are giving undefined behavior for cout << wontWork.notGood << endl;. I would retry that a non online compiler.

It is his fault for assuming that is has a default constructor, and it is a simple fix as well given that they should be able to understand the compiler error.

Yes it is but classes are supposed to be made to be as easy for the user(class user) to use as possible, meaning not limiting them on whatever they want to do as long as it is reasonable.

The only harm is making the assumption that a type has a default ctor. Not having a default ctor does not cause any harm.

We will agree to disagree on this I guess.

Which C++ Primer? There is a good one and a bad one. Also, could you provide page numbers and copy the exact argument/examples they give? EDIT: Oh, and which edition of Effective C++?

C++ Primer 4th Edition by Lippman and Moo. And Effective C++ 3rd Edition.


For pages in the primer

Page 482 Halfway down the page the text above best practices and the best practices.

Page 460

And the other pages on the constructor and copy constructor hold some nice info about them. But got most of the info from Effect C++ (Gezz I love those books)

As for Effect C++ 3rd Edition I don't have it with me at the moment but will let you know some page numbers in a hour or two when I get back to work (Yes I study at work ;p).

[LB]

Zereo wrote:

Umm did you? Because both codeblocks and VS2010 are giving undefined behavior for cout << wontWork.notGood << endl;. I would retry that a non online compiler.

This is starling, I've tried several compielrs and they all give different results o_o it also doesn't make sense, I thought that just writing "classname varname;" called the default ctor?

I'm definitely wrong, but I don't know how or why :\

Zereo wrote:

Yes it is but classes are supposed to be made to be as easy for the user(class user) to use as possible, meaning not limiting them on whatever they want to do as long as it is reasonable.

There are a large quantity of unreasonable reasons to have a default constructor, like with std::istream and std::ostream.


I'll look at those pages - I've definitely missed something. I definitely want to know why wontWork.notGood is undefined, because if it is undefined then it makes no sense for isGood.notGoot to be defined.

EDIT: adding a virtual function makes it undefined for IDEOne: http://ideone.com/Bz7n2X
O_O

[naraku9333]

@LB
I believe the problem is you are testing integrals, they (POD) don't have ctors. I believe the int i(0) notation is syntactic sugar (and nesescary for init lists)

[LB]

Ah - that makes sense; because it is entirely composed of primitives, it treats it like a C struct.

[Nobun]

I am not skilled enough to help you experts...
...however I have to say that, if you don't initialize a value for int (inside or outside a class) the behaviour inside gcc is that:
It will assign to a strange value (I think it displaies original value of allocated memory not touched by the var becouse user didn't provide any value).

infact, if you print

int a;
std::cout << a;

inside a main() test, and you try to compile, gcc will warn you that "a" is not initialized (but program will compile). I think the same will happen to unitialized values of a class (implicit costructor - when you don't define anyone - will allocate the memory for the integer, in the example, but I don't think it will also assign a default value).

I will not enter in the main argument.... too complex for my skills... hope I said something useful

[LB]

@Nobun yes, I had forgotten that it applied to POD structs disguised as classes.

[sveatlo]

I added default constructer to intialize all values to 0, but it didn't help. I also rewrited the whole program not using class, but it didn't help either. I think there must be something wrong with the IRC commands or the way they are send.
Does anybody know what's wrong?

[LB]

Have you tried using a networking library that handles socket communication for you? Normally programmers don't ever deal with raw sockets like in your code unless they are writing the library to do it.