Programming Forums > Application Development > C++

Soulstorm · May 6th, 2006, 2:13 PM

Question 1: I have made a simple "stash" class. It will hold a character array of variable size. This is my code (it may be a little messy. Recommendations about optimization are welcome

).

(Toggle Plain Text)

#ifndef STASH_H
#define STASH_H
#include <iostream>
#include <fstream>
using namespace std;

int gIncrement = 4;

class stash{
	char *ch;
	int currentSize; //position of the last character
	int currentStorage; //size of the entire array
	int next;
public:
	stash();
	stash(int startSize);
	
	~stash();
	
	void show(){
		cout << "currentStorage: " << currentStorage << "\n";
		cout << "current size: " << currentSize << "\n";
		cout << "Next: " << next << "\n";
		for(int i=0; i<currentSize; i++)
			cout << ch[i];
		cout << "\n";
	}
	
	void inflate();
	void add(char p);
	void insertString(char *p);
	char* returnAsString();
	void clearStash();
	
	void operator+(char *p);
	void operator=(char *p);
	
};

//**constructors and destructors
stash::stash(){
	ch = new char [0];
	currentSize = 0;
	currentStorage = 0;
	next = 0;
}

stash::stash(int startSize){
	ch = new char [startSize];
	currentSize = 0;
	currentStorage = startSize;
	next = currentStorage - currentSize;
}

stash::~stash(){
	delete [] ch;
	cout << "Freeing storage\n";
}
//**

//--Increase the stash's size to hold more chars.
void stash::inflate(){
	int i;
	char *temp = new char [currentSize];
	for(i=0; i<currentSize; i++)
		temp[i] = ch[i];
	ch = new char [currentStorage+ gIncrement];
	for(i=0; i<currentSize; i++)
		ch[i] = temp[i];
	currentStorage = currentStorage + gIncrement;
	delete [] temp;
}

//--add a character to the stash and resize it according
//--to space needed
void stash::add(char p){
	if(next == 0)
		inflate();
	ch[currentSize] = p;
	++currentSize;
	next = currentStorage - currentSize;
}

//--reset the stash
void stash::clearStash(){
	currentSize = 0;
	currentStorage = 0;
	ch = new char [0];
	next = currentStorage - currentSize;
}

//--insert an entire string into the stash
void stash::insertString(char *p){
	int i;
	for(i=0; p[i]; i++){
		add(p[i]);
	}
}

//--make the chars a string
char* stash::returnAsString(){
	add('\0');
	cout << ch;
	return ch;
}

//--add more chars into the string;
void stash::operator+(char *p){
	insertString(p);
}

//--clear the stash and hold a new string
void stash::operator=(char *p){
	clearStash();
	insertString(p);
}

#endif

#ifndef STASH_H
#define STASH_H
#include <iostream>
#include <fstream>
using namespace std;

int gIncrement = 4;

class stash{
	char *ch;
	int currentSize; //position of the last character
	int currentStorage; //size of the entire array
	int next;
public:
	stash();
	stash(int startSize);
	
	~stash();
	
	void show(){
		cout << "currentStorage: " << currentStorage << "\n";
		cout << "current size: " << currentSize << "\n";
		cout << "Next: " << next << "\n";
		for(int i=0; i<currentSize; i++)
			cout << ch[i];
		cout << "\n";
	}
	
	void inflate();
	void add(char p);
	void insertString(char *p);
	char* returnAsString();
	void clearStash();
	
	void operator+(char *p);
	void operator=(char *p);
	
};

//**constructors and destructors
stash::stash(){
	ch = new char [0];
	currentSize = 0;
	currentStorage = 0;
	next = 0;
}

stash::stash(int startSize){
	ch = new char [startSize];
	currentSize = 0;
	currentStorage = startSize;
	next = currentStorage - currentSize;
}

stash::~stash(){
	delete [] ch;
	cout << "Freeing storage\n";
}
//**

//--Increase the stash's size to hold more chars.
void stash::inflate(){
	int i;
	char *temp = new char [currentSize];
	for(i=0; i<currentSize; i++)
		temp[i] = ch[i];
	ch = new char [currentStorage+ gIncrement];
	for(i=0; i<currentSize; i++)
		ch[i] = temp[i];
	currentStorage = currentStorage + gIncrement;
	delete [] temp;
}

//--add a character to the stash and resize it according
//--to space needed
void stash::add(char p){
	if(next == 0)
		inflate();
	ch[currentSize] = p;
	++currentSize;
	next = currentStorage - currentSize;
}

//--reset the stash
void stash::clearStash(){
	currentSize = 0;
	currentStorage = 0;
	ch = new char [0];
	next = currentStorage - currentSize;
}

//--insert an entire string into the stash
void stash::insertString(char *p){
	int i;
	for(i=0; p[i]; i++){
		add(p[i]);
	}
}

//--make the chars a string
char* stash::returnAsString(){
	add('\0');
	cout << ch;
	return ch;
}

//--add more chars into the string;
void stash::operator+(char *p){
	insertString(p);
}

//--clear the stash and hold a new string
void stash::operator=(char *p){
	clearStash();
	insertString(p);
}

#endif

My problem is that I want to overload the operator ">>" so that I can give these commands:

(Toggle Plain Text)

stash s;
cin >> s;

I know some things about operator overloading, but I seem to have stuck because I can't figure out how to use the 'add' function inside the overloaded operator's function.

Can anyone give me some ideas?

Question 2:
Is there any way that I can use a 'cin' function to work with a string AND not being limited to the characters I give in the first word? For example:

(Toggle Plain Text)

string s;
cin >> s;

I want to have the string accept the string "Hello world" but it will only accept "hello". I could use cin.getline(), but that will limit my characters, because it takes as an argument a character array, so I must enter the maximum characters that I want to enter in advance.

I could also prompt the user with a message that about how many characters he will give and then use the 'new' and 'delete' keywords, but I want to avoid that.

Any workarounds?

OpenLoop · May 6th, 2006, 4:10 PM

Answer for Q2:

use cin.getline(s, sizeof(s));

Jimbo · May 6th, 2006, 6:19 PM

Dont make gIncrement a global variable. What if someone uses your code and sets it to -5? Also, growing your string by 4 isn't very effective if you're suddenly adding a lot to is (i.e. s += "supercalifragilisticexpialidocious" yields at least 8 calls to inflate(); you might consider an exponential growth such as doubling [a quick and easy one]). Furthermore, why use next when you could save yourself the memory and just do

(Toggle Plain Text)

if(currentStorage-currentSize == 0)

for overloading the operator>>, can't you just do something like:

(Toggle Plain Text)

istream& operator>>(istream& in, stash s)
{
   for(char ch = in.get(); ch != /* whatever you use for a delimiter */ && !in.fail(); ch = in.get()
      s.add(ch);
   return in;
}

It's been a long time since I've done this, but I think it'll work... :o

Seif · May 6th, 2006, 6:23 PM

alternative for question 2: getline(cin, s);

Ooble · May 6th, 2006, 7:41 PM

Quote:

Originally Posted by Seif

alternative for question 2: getline(cin, s);

That's the correct way to do it, I believe, as cin.getline expects a character array.

The Dark · May 6th, 2006, 7:56 PM

A few extra suggestions:
- You should make a copy constructor, otherwise if someone using your code does something like:

(Toggle Plain Text)

  stash a, b;
  a.insertstring("abc");
  b = a;

This will lead to both a and b having the same "ch" pointer, which will cause it to be deleted multiple times.

- The inflate function doesn't need to allocate memory for temp, copy the string into that, increase the ch memory and then copy it back. Also you are not deleting the original ch memory, causing a memory leak. An easier way is to allocate the new amount memory to the temp pointer, copy the ch memory into that, delete the ch memory, then copy the memory pointer from temp to ch.

- the returnAsString function changes the string value in such a way as to prevent any more add operations. This would be unexpected for a user. One solution is after adding in the zero byte, you could decrease the "next" value so that the next add operation will overwrite the zero byte. Another option is to always keep your ch string null terminated, that way you can just return the ch pointer. This way would mean that the returnAsString function could be a const function, which means you could pass const parameters to functions that wanted stash parameters.

- I'd prefer insertString to be called appendString as it is not clear from the function name where the new string would be inserted (.g. at the start or at the end).

Soulstorm · May 7th, 2006, 6:06 AM

Quote:

Originally Posted by The Dark

- the returnAsString function changes the string value in such a way as to prevent any more add operations. This would be unexpected for a user. One solution is after adding in the zero byte, you could decrease the "next" value so that the next add operation will overwrite the zero byte. Another option is to always keep your ch string null terminated, that way you can just return the ch pointer. This way would mean that the returnAsString function could be a const function, which means you could pass const parameters to functions that wanted stash parameters.

Thanks a lot. I hadn't noticed that bug.

Quote:

Originally Posted by The Dark

Also you are not deleting the original ch memory, causing a memory leak

I thought I did release it here:

(Toggle Plain Text)

stash::~stash(){
	delete [] ch;
	cout << "Freeing storage\n";
}
//**

Quote:

Use cin.getline(s, sizeof(s));

Actually, that didn't work, which makes much sence if you think that the compiler will need to know the size of the string in advance, but a string doesn't have a constant size. Also, this command expects a character array. I want it to accept a string. The correct way was "getline(cin, s)", as Seif said.

Quote:

Originally Posted by Jimbo

Dont make gIncrement a global variable. What if someone uses your code and sets it to -5? Also, growing your string by 4 isn't very effective if you're suddenly adding a lot to is (i.e. s += "supercalifragilisticexpialidocious" yields at least 8 calls to inflate(); you might consider an exponential growth such as doubling [a quick and easy one]). Furthermore, why use next when you could save yourself the memory and just do

Actually, in my main programs I will include code that will check this variable and make sure it has the correct values. But I will check for that 'doubling' method you say. Finally, the 'next' variable is used in many functions as you can see, and I will need it for some other things I want to do inside the class.

(Toggle Plain Text)

istream& operator>>(istream& in, stash s)
{
   for(char ch = in.get(); ch != /* whatever you use for a delimiter */ && !in.fail(); ch = in.get()
      s.add(ch);
   return in;
}

That didn't work. When I wrote

(Toggle Plain Text)

stash s;
cin >> s;
s.show();

whatever I may give, I get this output:

Quote:

Freeing storage
currentStorage: 0
current size: 0
Next: 0

Freeing storage

Which I don't know why it happens. The destructor is called twice. Why?

Here is my implementation:

(Toggle Plain Text)

class stash{
	char *ch;
	int currentSize; //position of the last character
	int currentStorage; //size of the entire array
	int next;
public:
	stash();
	stash(int startSize);
	
	~stash();
	
	void show(){
		cout << "currentStorage: " << currentStorage << "\n";
		cout << "current size: " << currentSize << "\n";
		cout << "Next: " << next << "\n";
		for(int i=0; i<currentSize; i++)
			cout << ch[i];
		cout << "\n";
	}
	
	void inflate();
	void add(char p);
	void insertString(char *p);
	char* returnAsString();
	void clearStash();
	
	void operator+(char *p);
	void operator=(char *p);
	friend istream& operator>>(istream& stream, stash s);

	
};

//..................functions functions functions

istream& operator>>(istream& stream, stash s){
	for(char ch = stream.get(); ch != '\n' && !stream.fail(); ch = stream.get())
		s.add(ch);
		return stream;
}

Any recommendations?

The Dark · May 7th, 2006, 8:35 AM

With the memory leak, I was taling about thi bit of code:

(Toggle Plain Text)

void stash::inflate(){
	int i;
	char *temp = new char [currentSize];
	for(i=0; i<currentSize; i++)
		temp[i] = ch[i];
	ch = new char [currentStorage+ gIncrement];
	for(i=0; i<currentSize; i++)
		ch[i] = temp[i];
	currentStorage = currentStorage + gIncrement;
	delete [] temp;
}

ch gets reassigned on the red line without first deleting the memory it points to.

Soulstorm · May 7th, 2006, 12:48 PM

Damn. I should have been more careful. Also, this is the new inflate function:

(Toggle Plain Text)

void stash::inflate(){
	int i;
	char *temp = new char [currentSize + gIncrement];
	for(i=0; i<currentSize; i++)
		temp[i] = ch[i];
	delete [] ch;
	ch = temp;
	currentStorage = currentStorage + gIncrement;
}

Now, can anyone tell me how am I going to make the operator overloading that I need? :o

Animatronic · May 7th, 2006, 1:19 PM

Your inflate function looks better now, maybe your'll want to allocate more meory than you actually need so that if you add to the string in the future you dont have to allocate again. e.g. std::string class normally rounds up to the nearest power of 2 : 16,32,64...

Jimbo made a small typo in the operator >> () but should have been easy to spot. If you want any function to work on an alias of an object, rather than a copy, you have to pass by reference.

so your operator should be:
std::istream& operator>> (std::istream& in, stash & s );

Also as the dark said you still need a copy constructor!

May 6th, 2006, 2:13 PM	#1
Soulstorm Hobbyist Programmer Join Date: Jan 2006 Location: Menidi, Athens, Greece Posts: 243 Rep Power: 3	Some questions: Question 1: I have made a simple "stash" class. It will hold a character array of variable size. This is my code (it may be a little messy. Recommendations about optimization are welcome ). (Toggle Plain Text) #ifndef STASH_H #define STASH_H #include <iostream> #include <fstream> using namespace std; int gIncrement = 4; class stash{ char ch; int currentSize; //position of the last character int currentStorage; //size of the entire array int next; public: stash(); stash(int startSize); ~stash(); void show(){ cout << "currentStorage: " << currentStorage << "\n"; cout << "current size: " << currentSize << "\n"; cout << "Next: " << next << "\n"; for(int i=0; i<currentSize; i++) cout << ch[i]; cout << "\n"; } void inflate(); void add(char p); void insertString(char p); char* returnAsString(); void clearStash(); void operator+(char p); void operator=(char p); }; //constructors and destructors stash::stash(){ ch = new char [0]; currentSize = 0; currentStorage = 0; next = 0; } stash::stash(int startSize){ ch = new char [startSize]; currentSize = 0; currentStorage = startSize; next = currentStorage - currentSize; } stash::~stash(){ delete [] ch; cout << "Freeing storage\n"; } // //--Increase the stash's size to hold more chars. void stash::inflate(){ int i; char temp = new char [currentSize]; for(i=0; i<currentSize; i++) temp[i] = ch[i]; ch = new char [currentStorage+ gIncrement]; for(i=0; i<currentSize; i++) ch[i] = temp[i]; currentStorage = currentStorage + gIncrement; delete [] temp; } //--add a character to the stash and resize it according //--to space needed void stash::add(char p){ if(next == 0) inflate(); ch[currentSize] = p; ++currentSize; next = currentStorage - currentSize; } //--reset the stash void stash::clearStash(){ currentSize = 0; currentStorage = 0; ch = new char [0]; next = currentStorage - currentSize; } //--insert an entire string into the stash void stash::insertString(char p){ int i; for(i=0; p[i]; i++){ add(p[i]); } } //--make the chars a string char* stash::returnAsString(){ add('\0'); cout << ch; return ch; } //--add more chars into the string; void stash::operator+(char p){ insertString(p); } //--clear the stash and hold a new string void stash::operator=(char p){ clearStash(); insertString(p); } #endif #ifndef STASH_H #define STASH_H #include <iostream> #include <fstream> using namespace std; int gIncrement = 4; class stash{ char ch; int currentSize; //position of the last character int currentStorage; //size of the entire array int next; public: stash(); stash(int startSize); ~stash(); void show(){ cout << "currentStorage: " << currentStorage << "\n"; cout << "current size: " << currentSize << "\n"; cout << "Next: " << next << "\n"; for(int i=0; i<currentSize; i++) cout << ch[i]; cout << "\n"; } void inflate(); void add(char p); void insertString(char p); char* returnAsString(); void clearStash(); void operator+(char p); void operator=(char p); }; //constructors and destructors stash::stash(){ ch = new char [0]; currentSize = 0; currentStorage = 0; next = 0; } stash::stash(int startSize){ ch = new char [startSize]; currentSize = 0; currentStorage = startSize; next = currentStorage - currentSize; } stash::~stash(){ delete [] ch; cout << "Freeing storage\n"; } // //--Increase the stash's size to hold more chars. void stash::inflate(){ int i; char temp = new char [currentSize]; for(i=0; i<currentSize; i++) temp[i] = ch[i]; ch = new char [currentStorage+ gIncrement]; for(i=0; i<currentSize; i++) ch[i] = temp[i]; currentStorage = currentStorage + gIncrement; delete [] temp; } //--add a character to the stash and resize it according //--to space needed void stash::add(char p){ if(next == 0) inflate(); ch[currentSize] = p; ++currentSize; next = currentStorage - currentSize; } //--reset the stash void stash::clearStash(){ currentSize = 0; currentStorage = 0; ch = new char [0]; next = currentStorage - currentSize; } //--insert an entire string into the stash void stash::insertString(char p){ int i; for(i=0; p[i]; i++){ add(p[i]); } } //--make the chars a string char* stash::returnAsString(){ add('\0'); cout << ch; return ch; } //--add more chars into the string; void stash::operator+(char p){ insertString(p); } //--clear the stash and hold a new string void stash::operator=(char p){ clearStash(); insertString(p); } #endif My problem is that I want to overload the operator ">>" so that I can give these commands: (Toggle Plain Text) stash s; cin >> s; stash s; cin >> s; I know some things about operator overloading, but I seem to have stuck because I can't figure out how to use the 'add' function inside the overloaded operator's function. Can anyone give me some ideas? Question 2: Is there any way that I can use a 'cin' function to work with a string AND not being limited to the characters I give in the first word? For example: (Toggle Plain Text) string s; cin >> s; string s; cin >> s; I want to have the string accept the string "Hello world" but it will only accept "hello". I could use cin.getline(), but that will limit my characters, because it takes as an argument a character array, so I must enter the maximum characters that I want to enter in advance. I could also prompt the user with a message that about how many characters he will give and then use the 'new' and 'delete' keywords, but I want to avoid that. Any workarounds? __________________ Project::Soulstorm (personal homepage) Last edited by Soulstorm; May 6th, 2006 at 2:28 PM.

May 6th, 2006, 6:19 PM	#3
Jimbo Battle Programmer Join Date: Feb 2006 Location: Bellevue, WA, USA Posts: 770 Rep Power: 3	Dont make gIncrement a global variable. What if someone uses your code and sets it to -5? Also, growing your string by 4 isn't very effective if you're suddenly adding a lot to is (i.e. s += "supercalifragilisticexpialidocious" yields at least 8 calls to inflate(); you might consider an exponential growth such as doubling [a quick and easy one]). Furthermore, why use next when you could save yourself the memory and just do (Toggle Plain Text) if(currentStorage-currentSize == 0) if(currentStorage-currentSize == 0) for overloading the operator>>, can't you just do something like: (Toggle Plain Text) istream& operator>>(istream& in, stash s) { for(char ch = in.get(); ch != /* whatever you use for a delimiter / && !in.fail(); ch = in.get() s.add(ch); return in; } istream& operator>>(istream& in, stash s) { for(char ch = in.get(); ch != / whatever you use for a delimiter */ && !in.fail(); ch = in.get() s.add(ch); return in; } It's been a long time since I've done this, but I think it'll work... :o

May 6th, 2006, 7:56 PM	#6
The Dark Expert Programmer Join Date: Jun 2005 Posts: 893 Rep Power: 4	A few extra suggestions: - You should make a copy constructor, otherwise if someone using your code does something like: (Toggle Plain Text) stash a, b; a.insertstring("abc"); b = a; stash a, b; a.insertstring("abc"); b = a; This will lead to both a and b having the same "ch" pointer, which will cause it to be deleted multiple times. - The inflate function doesn't need to allocate memory for temp, copy the string into that, increase the ch memory and then copy it back. Also you are not deleting the original ch memory, causing a memory leak. An easier way is to allocate the new amount memory to the temp pointer, copy the ch memory into that, delete the ch memory, then copy the memory pointer from temp to ch. - the returnAsString function changes the string value in such a way as to prevent any more add operations. This would be unexpected for a user. One solution is after adding in the zero byte, you could decrease the "next" value so that the next add operation will overwrite the zero byte. Another option is to always keep your ch string null terminated, that way you can just return the ch pointer. This way would mean that the returnAsString function could be a const function, which means you could pass const parameters to functions that wanted stash parameters. - I'd prefer insertString to be called appendString as it is not clear from the function name where the new string would be inserted (.g. at the start or at the end).

May 7th, 2006, 8:35 AM	#8
The Dark Expert Programmer Join Date: Jun 2005 Posts: 893 Rep Power: 4	With the memory leak, I was taling about thi bit of code: (Toggle Plain Text) void stash::inflate(){ int i; char temp = new char [currentSize]; for(i=0; i<currentSize; i++) temp[i] = ch[i]; ch = new char [currentStorage+ gIncrement]; for(i=0; i<currentSize; i++) ch[i] = temp[i]; currentStorage = currentStorage + gIncrement; delete [] temp; } void stash::inflate(){ int i; char temp = new char [currentSize]; for(i=0; i<currentSize; i++) temp[i] = ch[i]; ch = new char [currentStorage+ gIncrement]; for(i=0; i<currentSize; i++) ch[i] = temp[i]; currentStorage = currentStorage + gIncrement; delete [] temp; } ch gets reassigned on the red line without first deleting the memory it points to.

May 7th, 2006, 12:48 PM	#9
Soulstorm Hobbyist Programmer Join Date: Jan 2006 Location: Menidi, Athens, Greece Posts: 243 Rep Power: 3	Damn. I should have been more careful. Also, this is the new inflate function: (Toggle Plain Text) void stash::inflate(){ int i; char temp = new char [currentSize + gIncrement]; for(i=0; i<currentSize; i++) temp[i] = ch[i]; delete [] ch; ch = temp; currentStorage = currentStorage + gIncrement; } void stash::inflate(){ int i; char temp = new char [currentSize + gIncrement]; for(i=0; i<currentSize; i++) temp[i] = ch[i]; delete [] ch; ch = temp; currentStorage = currentStorage + gIncrement; } Now, can anyone tell me how am I going to make the operator overloading that I need? :o __________________ Project::Soulstorm (personal homepage)

May 6th, 2006, 4:10 PM	#2
OpenLoop Expert Programmer Join Date: May 2005 Location: East Lansing, MI Posts: 663 Rep Power: 4	Answer for Q2: use cin.getline(s, sizeof(s));

May 6th, 2006, 6:23 PM	#4
Seif Hobbyist Programmer Join Date: Jan 2006 Location: UK Posts: 242 Rep Power: 3	alternative for question 2: getline(cin, s);

May 7th, 2006, 1:19 PM	#10
Animatronic Programmer Join Date: Jun 2005 Posts: 99 Rep Power: 4	Your inflate function looks better now, maybe your'll want to allocate more meory than you actually need so that if you add to the string in the future you dont have to allocate again. e.g. std::string class normally rounds up to the nearest power of 2 : 16,32,64... Jimbo made a small typo in the operator >> () but should have been easy to spot. If you want any function to work on an alias of an object, rather than a copy, you have to pass by reference. so your operator should be: std::istream& operator>> (std::istream& in, stash & s ); Also as the dark said you still need a copy constructor!

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Programming Forums > Application Development > C++

Some questions: