why does the program crash?

[maharjun]

i tried making a string class but my program seems to be repeatedly crashing. why so?

I have the following two files in to my program

1.)Strng.h (in INCLUDE directory)

#ifndef __STRNG_H
#define __STRNG_H

#if !defined(__CONIO_H)
#include <conio.h>
#endif

#if !defined(__STRING_H)
#include <string.h>
#endif

#if !defined(__STDDEF_H)
#include <stddef.h>
#endif

//#if !defined(__IOSTREAM_H)
//#include <iostream.h>
//#endif

#if !defined(__STDIO_H)
#include <stdio.h>
#endif

class String
{
    public:
	char *string;

    public: String(char *str = "")
    {
	(*this).assignstring(str);
    }
    ~String(){delete string;};
    public: void assignstring(char *str)
    {
	delete string;
	string = new char[strlen(str)+1];
	if(string != NULL)
	    strcpy((*this).string,str);
    }

    public: String operator=(char* str)
    {
	(*this).assignstring(str);
	return *this;
    }

    public: String operator +(String s)
    {
	String cat("");

	int len = strlen((*this).string)+strlen((s.string))+1;
	char *a = new char[len];
	if(a != NULL)
	{
	    strcpy(a,(*this).string);
	    strcat(a,s.string);
	    cat.assignstring(a);
	    delete a;
	}
	else
	{
	    printf("illegal operation");
	}
	return cat;
    }
};
#endif

2.) String2.cpp (in BIN directory)

#include <iostream.h>
#include <conio.h>
#include <string.h>
#include <Strng.h>


void main()
{
    clrscr();
    String str(""),str2(""),str3("");

    str = "hello world";
    str2 = " howdy world";
    str3 = str2+(str+str);

    cout<<str3.string;
}

[jonsca]

A few preliminaries: <iostream.h> has long been standardized to <iostream> . When using string.h with C++ you should include it as <cstring> . Also the conio routine is non-standard. Finally, main must return an int according to the standard.

You need to change your = operator to look something like this:

public: String & operator=(const String & str)
    {
	(*this).assignstring(str.string); //or this->assignstring(str.string);
	return *this;
    }

You were trying to equate a String (your class) object to a char * which wouldn't work. I would take out line 36. If you're redefining string (your member variable) again on line 37 I don't believe you need to delete the old one first, someone can probably clarify that for you. I think having your destructor do that when all is said and done is enough.

Just a suggestion. I know this is a standalone project and you may never see it again but it's a good habit to get away from naming things after existing entities (e.g. string vs. std::string in particular but String could turn into string with a typo) unless you explicitly plan on putting this into a namespace and qualifying the names.

[VilePlecenta]

Also since its a local include, shouldn't "Strng.h" be with quotations....

[jonsca]

He's parked it in the includes directory on the search path for his compiler. I thought that too at first.

[mattjbond]

You have further issues with this code. Your main problem is centered around poor design, as previously stated yoiu have not correctly defined your assignment operator=, but in addition to this you should also define a copy constructor of the form -

String( const String& str )
{
   string = new char[strlen(str.string)+1];
   strcpy( string, str.string );
}

When you write code which is passing around objects by value as you have done here -

String operator+(String s) // this passes a temporary to the function

the copy constructor is invoked to created the temporaries requried to do this. As you have not defined one, the implicit compiler generated version is used (the compiler will implicitly create a default constructor, copy constructor and assignment operator for you if one is not defined). These implicit versions do only a memberwise copy of the object, meaning they copy the object member by member by member. In your case this is not correct behaviour as they should copy the allocated memory too. You can avoid passing of temporaries by passing variables as references. this wont help you here though as the lack of copy constructor is a problem.

There are loads of other issues with this code that I do not have time to adequately explain. It is not exception safe for instance. Why are you trying to define a string class anyway when the standard provides one?
Without wanting to sound like a tosser, i'd suggest you get a good beginner level book and study it from the start. I noticed that one of the threads at the top of this forum has a subject of books. After a quick look I can say that all the books mentioned in the original post are excellent but I wouldn't go too far through the rest of the posts. One book I might have added is Essential C++ by Stan Lippman, who wrote the C++ Primer 4th edition which is mentioned. This is a nice short text going over the essentials which can be used as an introductory text before moving on to more detailed books.

[jonsca]

It is not exception safe for instance. Why are you trying to define a string class anyway when the standard provides one?

Without making too many assumptions this is probably a homework assignment for an intro class (where reinvention usually reigns supreme). We could probably debate the exceptions issue ad nauseum but I don't think the OP would benefit from that at this stage.

I certainly agree with the rest of your points.

[mattjbond]

Without making too many assumptions this is probably a homework assignment for an intro class (where reinvention usually reigns supreme). We could probably debate the exceptions issue ad nauseum but I don't think the OP would benefit from that at this stage.

I certainly agree with the rest of your points.

To be honest I didn't have time to list all my issues with this code, we'd have been here for quite some time, but I would take exception at any C++ course which was attempting to teach using a mishmash of standard C++ and C style arrays. Whilst they have their uses why not advocate the use of std::string and std::vector etc. right from the start? If this is homework I think i'd complain about its quality.

[mitrmkar]

In addition to what's been already stated. Only focusing on a very basic thing.

This basic thing here is that you try to delete memory that was never allocated in the first place. In other words, you do delete string without having allocated any memory to string and string is uninitialized at the point of deletion.

Try debugging the following code step-by-step, and you'll see how it goes wrong in the assignstring() method.

int main()
{
  String oops("ouch");
}

So, one basic thing for you to do, is to figure out how to protect the code from this kind of crashing (hint: it's about initializing variables prior to their usage).

PS. If you do new some_thing[some_size]; then you must delete [] some_thing; or else your program leaks memory.

[mattjbond]

In addition to what's been already stated. Only focusing on a very basic thing.

This basic thing here is that you try to delete memory that was never allocated in the first place. In other words, you do delete string without having allocated any memory to string and string is uninitialized at the point of deletion.

Try debugging the following code step-by-step, and you'll see how it goes wrong in the assignstring() method.

int main()
{
  String oops("ouch");
}

So, one basic thing for you to do, is to figure out how to protect the code from this kind of crashing (hint: it's about initializing variables prior to their usage).

PS. If you do new some_thing[some_size]; then you must delete [] some_thing; or else your program leaks memory.

Agreed. Thought I had mentioned this too, but apparently deleted it whilst editing post. Oops.