#include <iostream>
using namespace std;
class string_
{
char* c;
public:
string_ (char* c);
~string_ ();
// Methods
int get_length();
char* display();
char* assign(string_ &s);
int compare (string_ &s);
char* concatenate(string_ &s);
int find_index(char str);
};
string_::string_ (char* s)
{
c = new char[strlen(s)];
strcpy(c, s);
}
string_::~string_ ()
{
delete []c;
}
/
**********************************************************************************************/
inline int string_::get_length ()
{
return strlen (c);
}
inline char* string_::assign (string_ &s)
{
char *str = new char[strlen(s.c)+ 1];
return strcpy (str, s.c);
}
inline char* string_::display()
{
return c;
}
inline int string_::compare (string_ &s)
{
if (!strcmp (c, s.c))
return 1;
else
return 0;
}
inline char* string_::concatenate(string_ &s)
{
return strcat(c, s.c);
}
int string_::find_index (char str)
{
int i = 0;
while (c[i]!= '\0')
if(c[i]== str)
return i;
else
i++;
return -1;
}
int main()
{
string_ s1 ("Hello");
string_ s2("test");
char c_char;
// 1- length
cout << "1- Get length: \n";
cout << "s1 = \"" << s1.display() << "\"\n";
cout << "Length of s1 is: "<<s1.get_length () << " characters" <<
"\n";
cout << "s2 = \"" << s2.display() << "\"\n";
cout << "Length of s2 is: "<<s2.get_length () << " characters" << "\n
\n";
// 2- Assign
cout << "2- Assign \"s1\" to \"s2\":\n";
cout << "Before: s2 = \"" << s2.display() << "\"\n";
cout << "After :s2 = " << s2.assign(s1) << "\n\n";
// 3- Compare
cout << "3- Compare two strings:\n";
cout << "s1 = \"" << s1.display() << "\", ";
cout << "s2 = \"" << s2.display() << "\"\n";
if (s1.compare(s2))
cout << "s1 and s2 are equal\n";
else
cout << "s1 and s2 are different\n\n";
// 4- Concatenate
cout << "4- Concatenate s1 to s2:\n";
cout << "s1 = \"" << s1.display() << "\", ";
cout << "s2 = \"" << s2.display() << "\"\n";
cout << "Now s2 is \"" << s2.concatenate(s1) << "\"\n\n";
// 5- Find index
cout << "5- Find index in s1:\n";
cout << "Enter a charcter:\n";
cin >> c_char;
int i = s1.find_index (c_char);
if (i == -1)
cout << "Character \"" << c_char << "\" does not exist in \"" <<
s1.display() << "\"\n";
else
cout << "Index of \"" << c_char << "\" in \"" << s1.display() << "\"
is :" << i << "\n";
cout << "\nEnd of program\n";
return 0;
}
After excution I have:
1- Get length:
s1 = "Hello"
Length of s1 is: 5 characters
s2 = "test"
Length of s2 is: 4 characters
2- Assign "s1" to "s2":
Before: s2 = "test"
After :s2 = Hello ///---->> Here s2
contains "hello"
3- Compare two strings:
s1 = "Hello", s2 = "test" ///---->> So why here s2
contains the old string "test"?
s1 and s2 are different
4- Concatenate s1 to s2:
s1 = "Hello", s2 = "test"
Now s2 is "testHello"
5- Find index in s1:
Enter a charcter:
c
Character "c" does not exist in "Hello"
End of program
malloc/free: in use at exit: 0 bytes in 1 blocks.
malloc/free: 2 allocs, 2 frees, 5 bytes allocated.
LEAK SUMMARY:
==15376== definitely lost: 0 bytes in 1 blocks.
==15376== possibly lost: 0 bytes in 0 blocks.
==15376== still reachable: 0 bytes in 0 blocks.
==15376== suppressed: 0 bytes in 0 blocks.
1 block is losted : does it mean that I have a block of memory not yet
freed ?
> Hi all,
> In the next program I wrote class "string_ " with next functions to
> manipulate strings : assign, compare, concatenate, find index of
> character.
> 1- Is there any method to write these functions without using C-
> standard functions, i.e to do not use #include <string.h> ?
Sure.
> 2- After assigning s1 to s2 in the next program, the content of s2
> disappears when I try to use it in other function. Why this occurs?
Your assignment method is messed up. More details below.
> should I use a static variable ?
No. (and: why?)
> #include <iostream>
> using namespace std;
>
> class string_
> {
> char* c;
> public:
> string_ (char* c);
> ~string_ ();
>
> // Methods
> int get_length();
> char* display();
> char* assign(string_ &s);
> int compare (string_ &s);
> char* concatenate(string_ &s);
> int find_index(char str);
> };
>
> string_::string_ (char* s)
> {
> c = new char[strlen(s)];
You have an off-by-one error here.
> strcpy(c, s);
> }
>
> string_::~string_ ()
> {
> delete []c;
> }
>
> /
>
**********************************************************************************************/
> inline int string_::get_length ()
> {
> return strlen (c);
> }
>
> inline char* string_::assign (string_ &s)
> {
> char *str = new char[strlen(s.c)+ 1];
> return strcpy (str, s.c);
You never change the pointer this->c. You also never change the string it
points to. No assignment is made, but the new string is returned. (Unless
the caller deletes the string, the memory will leak.)
BTW: it would be more natural to return a string_& and end with
return ( *this );
> }
>
> inline char* string_::display()
> {
> return c;
> }
>
> inline int string_::compare (string_ &s)
> {
> if (!strcmp (c, s.c))
> return 1;
> else
> return 0;
> }
If you don't do a three-way comparison, you should return a bool.
>
> inline char* string_::concatenate(string_ &s)
> {
> return strcat(c, s.c);
You have undefined behavior: strcat() assumes that the destination has
enough memory. That is false in your class.
> }
>
> int string_::find_index (char str)
> {
> int i = 0;
> while (c[i]!= '\0')
> if(c[i]== str)
> return i;
> else
> i++;
> return -1;
> }
[snip]
Best
Kai-Uwe Bux
#include <string.h>
using namespace std;
class string_
{
char* c;
public:
string_ (char* c);
~string_ ();
// Methods
int get_length();
char* display();
char* assign(string_ &s);
bool compare (string_ &s);
char* concatenate(string_ &s);
int find_index(char str);
};
string_::string_ (char* s)
{
c = new char[strlen(s)];
strcpy(c, s);
}
string_::~string_ ()
{
delete []c;
}
/
************************************************************************************************
// * Functions
// *
**********************************************************************************************/
inline int string_::get_length ()
{
return strlen (c);
}
inline char* string_::assign (string_ &s)
{
strcpy(this -> c,s.c); // Modify object pointer
return this ->c;
}
inline char* string_::display()
{
return this -> c;
}
inline bool string_::compare (string_ &s)
{
if (!strcmp (c, s.c))
return true;
else
return false;
}
char* string_::concatenate(string_ &s)
{
strcat(this -> c, s.c);
return this -> c;
}
int string_::find_index (char str)
{
int i = 0;
while (c[i]!= '\0')
if(c[i]== str)
return i;
else
i++;
return -1;
}
int main()
{
string_ s1 ("Hello");
string_ s2("test");
char c_char;
// 1- length
cout << "1- Get length: \n";
cout << "s1 = \"" << s1.display() << "\"\n";
cout << "Length of s1 is: "<<s1.get_length () << " characters" <<
"\n";
cout << "s2 = \"" << s2.display() << "\"\n";
cout << "Length of s2 is: "<<s2.get_length () << " characters" << "\n
\n";
// 2- Assign
cout << "2- Assign \"s1\" to \"s2\":\n";
cout << "Before: s2 = \"" << s2.display() << "\"\n";
cout << "After :s2 = " << s2.assign(s1) << "\n\n";
// 3- Compare
cout << "3- Compare two strings:\n";
cout << "s1 = \"" << s1.display() << "\", ";
cout << "s2 = \"" << s2.display() << "\"\n";
if (s1.compare(s2)== true)
cout << "s1 and s2 are equal\n\n";
else
cout << "s1 and s2 are different\n\n";
// 4- Concatenate
cout << "4- Concatenate s1 to s2:\n";
cout << "s1 = \"" << s1.display() << "\", ";
cout << "s2 = \"" << s2.display() << "\"\n";
cout << "Now s2 is \"" << s2.concatenate(s1) << "\"\n\n";
// 5- Find index
cout << "5- Find index in s1:\n";
cout << "Enter a charcter:\n";
cin >> c_char;
int i = s1.find_index (c_char);
if (i == -1)
cout << "Character \"" << c_char << "\" does not exist in \"" <<
s1.display() << "\"\n";
else
cout << "Index of \"" << c_char << "\" in \"" << s1.display() << "\"
is :" << i << "\n";
cout << "\nEnd of program\n";
return 0;
}
Program is working. after I tested by valgring to check for memory
leak. As result I get:
malloc/free: in use at exit: 0 bytes in 0 blocks.
==18833== malloc/free: 2 allocs, 2 frees, 9 bytes allocated.
==18833==
==18833== All heap blocks were freed -- no leaks are possible.
--18833-- memcheck: sanity checks: 2 cheap, 1 expensive
--18833-- memcheck: auxmaps: 179 auxmap entries (11456k, 11M) in use
--18833-- memcheck: auxmaps: 494745 searches, 940993 compariso
....
Should I consider that is 100 % right ?
> I've modified my code like this:
>
> #include <string.h>
> using namespace std;
>
> class string_
> {
> char* c;
> public:
> string_ (char* c);
> ~string_ ();
>
> // Methods
> int get_length();
> char* display();
> char* assign(string_ &s);
> bool compare (string_ &s);
> char* concatenate(string_ &s);
> int find_index(char str);
> };
>
> string_::string_ (char* s)
> {
> c = new char[strlen(s)];
> strcpy(c, s);
You still are off by one.
> }
>
>
> string_::~string_ ()
> {
> delete []c;
> }
>
> /
>
************************************************************************************************
> // * Functions
> // *
>
**********************************************************************************************/
> inline int string_::get_length ()
> {
> return strlen (c);
> }
>
> inline char* string_::assign (string_ &s)
> {
> strcpy(this -> c,s.c); // Modify object pointer
This will write to unallocated memory if this->c is shorter than s.c.
> return this ->c;
> }
>
> inline char* string_::display()
> {
> return this -> c;
> }
>
>
> inline bool string_::compare (string_ &s)
> {
> if (!strcmp (c, s.c))
> return true;
> else
> return false;
> }
>
> char* string_::concatenate(string_ &s)
> {
> strcat(this -> c, s.c);
This will write to unallocated memory whenever s.c has positive length.
> return this -> c;
> }
>
> int string_::find_index (char str)
> {
> int i = 0;
> while (c[i]!= '\0')
> if(c[i]== str)
> return i;
> else
> i++;
> return -1;
> }
[snip]
>
> Program is working. after I tested by valgring to check for memory
> leak. As result I get:
>
> malloc/free: in use at exit: 0 bytes in 0 blocks.
> ==18833== malloc/free: 2 allocs, 2 frees, 9 bytes allocated.
> ==18833==
> ==18833== All heap blocks were freed -- no leaks are possible.
> --18833-- memcheck: sanity checks: 2 cheap, 1 expensive
> --18833-- memcheck: auxmaps: 179 auxmap entries (11456k, 11M) in use
> --18833-- memcheck: auxmaps: 494745 searches, 940993 compariso
> ....
>
> Should I consider that is 100 % right ?
No, there are still at least three problems with the memory handling in the
code.
Best
Kai-Uwe Bux
inline char* string_::assign (string_ &s)
{
char *str = new char[strlen(s.c)+ 1];
strcpy (str, s.c);
delete []c;
c = str;
return this -> c;
}
and:
inline char* string_::assign (string_ &s)
{
char *str = new char[strlen(s.c)+ 1];
strcpy (str, s.c);
delete []c;
c = str;
return this -> c;
}
Now I think that there no problem with memory leak
Reslut of valgrind:
malloc/free: in use at exit: 0 bytes in 0 blocks.
==21224== malloc/free: 4 allocs, 4 frees, 26 bytes allocated.
==21224==
==21224== All heap blocks were freed -- no leaks are possible.
--21224-- memcheck: sanity checks: 2 cheap, 1 expensive
--21224-- memcheck: auxmaps: 179 auxmap entries (11456k, 11M) in use
--21224-- memcheck: auxmaps: 495363 searches, 941649 comparisons
....
> Ok... I changed two functions: asisgn and concatenate.
>
> inline char* string_::assign (string_ &s)
> {
> char *str = new char[strlen(s.c)+ 1];
> strcpy (str, s.c);
> delete []c;
> c = str;
> return this -> c;
> }
That looks right.
> and:
>
> inline char* string_::assign (string_ &s)
> {
> char *str = new char[strlen(s.c)+ 1];
> strcpy (str, s.c);
> delete []c;
> c = str;
> return this -> c;
> }
That's just the function assign once more.
BTW: Did you see fix off-by-1 error in the constructor?
Best
Kai-Uwe Bux
Why don't you use std::string? It will save you tons of trouble.
> class string_
> {
> char* c;
> public:
> string_ (char* c);
> ~string_ ();
>
> // Methods
> int get_length();
> char* display();
> char* assign(string_ &s);
> int compare (string_ &s);
> char* concatenate(string_ &s);
> int find_index(char str);
> };
What do you think happens with code like this:
void foo()
{
string_ s1 = something;
string_ s2 = s1; // What happens here?
} // What happens when the function ends?
Also:
void bar(string_ s)
{
}
void foo()
{
string_ s = something;
bar(s);
s.get_length(); // What happens here?
}
Sorry I pasted 2 times the same code.
So the second function is :
char* string_::concatenate(string_ &s)
{
char *conc = new char [strlen(c) + strlen(s.c)+1];
strcpy(conc, c);
strcat(conc, s.c);
delete []c;
c = conc;
return this -> c;
}
[snip]
> Sorry I pasted 2 times the same code.
> So the second function is :
> char* string_::concatenate(string_ &s)
> {
> char *conc = new char [strlen(c) + strlen(s.c)+1];
> strcpy(conc, c);
> strcat(conc, s.c);
> delete []c;
> c = conc;
> return this -> c;
> }
Yup, that would do it.
BTW: I hope, this is an exercise in hands on memory management. In real
life, you should use std::string for strings. Also, manual memory
management is best avoided, since it is difficult and error prone. It
usually pays to spend a lot of brain cycles during the design phase on
avoiding memory management issues so that one can save many more brain
cycles during coding.
Best
Kai-Uwe Bux
And use this:
char* string_::concatenate(string_ &s)
{
strcat(c, s.c);
return this -> c;
}
I think that we can't do this, because in the case when length in bits
of c is less than length of S.c, there is not enough space in bits to
concatenate these two strings.
Am I right ?
I have a hard time making sense of "length in bits of c < length of S.c".
But you are right that what matters is whether the space reserved in c is
large enough to hold the concatenation.
> Am I right ?
You are right. Especially with your string_ class the buffer this->c is
always tight: It is allocated to hold the string and the terminating 0 in
the constructor (you fixed the off-by-one thingy, right?) and all methods
preserve that feature. Thus, it is not large enough to hold more. On the
other hand, strcat() _assumes_ that the target buffer is big enough to hold
the concatenation. This hypothesis is _always_ violated as per an invariant
of the string_ class when S.c is non-empty.
Best
Kai-Uwe Bux
I modified the constructor to hold the the character '\0' like this:
string_::string_ (char* s)
{
c = new char[strlen(s) + 1];
strcpy(c, s);
}
Yup, that's exactly what I meant :-)
Best
Kai-Uwe Bux
Correct.
Afterwards, you should lern about "const correctness", and add a copy
constructor and a copy assignment operator to your class.
--
Thomas
I advise you to reread Juha's post.
In particular, have you tried this?
int main()
{
{
string_ s1 = "hello"
string_ s2 = s1;
}
}
I tried your code and have as result:
s2 = "hello";
That's one possible outcome (in fact a likely one). Nonetheless, you have
undefined behavior.
What you see is a compiler generated copy constructor and assignment
operator at work, since you did not provide them yourself. The compiler
generated assignment operator works member by member and performs
assignments there. The compiler generated copy constructor also recurses on
the members and uses the copy constructor of each member for that member.
For your class, there is only one member:
class string_ {
char* c;
public:
...
};
Now what happens if a string_ str_b is copy constructed from another string_
object, say, str_a? Well, the member str_b.c is constructed from str_a.c.
That is a char* and it will _not cause allocation_. You end up with two
pointers to the _same_ address. Similar remarks apply to the assignment
operator. This is why you observe s2 = "hello". The char* s2.c points to
the same memory location as s1.c, and "hello" happens to reside there.
The catch happens when str_a or str_b or some intermediate temporary with
the same char* member is destroyed. Have a look at your destructor:
string_::~string_ () {
delete []c;
}
If two objects str_a and str_b have the same pointer, i.e., if
str_a.c == str_b.c
then destruction of the first object will ensure that the memory is
deallocated. The remaining object now has an invalid pointer member. Any
operation that dereferences this pointer is undefined behavior. But that's
not all: when the second object is destroyed, the same memory is deleted
another time and you have undefined behavior again because of double
deletion.
Therefore, you should provide a copy constructor that allocates the needed
memory. Your assign() method provided the right blueprint for a good
assignment operator.
Best
Kai-Uwe Bux
Here is my code:
#include <iostream>
#include <string.h>
using namespace std;
class string_
{
char* c;
public:
string_ ();
string_ (char* c);
string_ (const string_ & old_string_);
~string_ ();
// Methods
int get_length();
char* display();
char* assign(string_ &s);
char* assign(char* s_c);
bool compare (string_ &s);
char* concatenate(string_ &s);
int find_index(char str);
};
// Null constructor
string_::string_ (){}
// Constructor
string_::string_ (char* s)
{
c = new char[strlen(s)+1];
strcpy(c, s);
}
// Copy constructor
string_::string_(const string_ & old_string_)
{
char *tmp;
tmp = new char[strlen(old_string_.c)+1];
strcpy (tmp, old_string_.c);
c = tmp;
}
string_::~string_ ()
{
delete []c;
}
/
************************************************************************************************
// * Functions
// *
**********************************************************************************************/
inline int string_::get_length ()
{
return strlen (c);
}
char* string_::assign(char* s_c)
{
c = s_c;
return c;
}
//inline char* string_::assign (string_ &s)
char* string_::assign (string_ &s)
{
char *str = new char[strlen(s.c)+ 1]; // can't unuse str because,
What if strlen(s.c) > strlen(c)?
strcpy (str, s.c);
delete []c;
c = str;
return c;
}
char* string_::display()
{
return c;
}
bool string_::compare (string_ &s)
{
if (!strcmp (c, s.c))
return true;
else
return false;
}
char* string_::concatenate(string_ &s)
{
char *conc = new char [strlen(c) + strlen(s.c)+1];
strcpy(conc, c);
strcat(conc, s.c);
delete []c;
c = conc;
return c;
}
int string_::find_index (char str)
{
int i = 0;
while (c[i]!= '\0')
if(c[i]== str)
return i;
else
i++;
return -1;
}
void foo();
void bar(string_ s);
void foo_1();
// 6- Create a copy of s1 using copy constructor
string_ s3(s1);
cout << "\n6- Content of s3 is: " << s3.display() << endl;
cout << "test foo: \n"; foo();
// 7- Test of foo_1
cout << "\nTest of foo_1" << endl;
foo_1();
//8 Test copy assign
string_ s6 ("test s6");
s6.assign("reassign s6"); // Here compiler uses assign copy
cout << s6.display() << endl;
cout << "\nEnd of program\n";
return 0;
}
void foo()
{
string_ s4 = "HI"; // Use of constructor string_(char *s)
string_ s5 = s4; // Use of copy constructor to create s5
cout << "s4 is: " << s4.display() << endl;
cout << "s5 is: " << s5.display() << endl;
} // In this function compiler used copy constructor to create a new
string_ class variable
void bar(string_ s)
{
cout << "Fom bar function: s is: " << s.display() << endl;
}
void foo_1()
{
string_ s = "There"; // Use of constructor string_(char *s)
bar(s); // To pass s to another function, compiler uses
copy constructor to pass s to function bar()
cout << "Length of string_ s is: " << s.get_length() << endl; //
Here simply use of member methods of class string_
}