• Advertisement
Sign in to follow this  

Unity critique my string class

This topic is 3975 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello, Community During the last few days, I've been writing a string class for c++. This is only as an experiment to see if it can be done. For me, it's kind of annoying working with stuff that I have no clue as how it works. So, I wrote this class as an experiment. Anyway, I would like some constructive criticism regarding this class.
class CODE_SYMBOL_PORT String
{
public:
	String();
	String(const String& String);
	String(const Char* String);
	//String(const Char* String, UInt32 Start, UInt32 Count);
	virtual ~String();

	Char *GetString() const;
	const UInt32 GetLength() const;
	const bool IsNull() const;
	const bool IsValidIndex(const UInt32 Index) const;

	UInt32 Compare(const String& String, bool IgnoreCase=false) const;
	bool Find(String& Str, UInt32& Pos, bool IgnoreCase=false) const;
	void Delete(UInt32 Pos, UInt32 Count);
	void Insert(UInt32 Pos, Char c);
	void Insert(UInt32 Pos, const String& String);
	void ClipLeadingWhitespace();
	void ClipTrailingWhitespace();
	void Trim();
	UInt32 ToInt() const;

	void operator =(const String& String);
	void operator =(const Char *String);
	void operator +=(const String& String);
	operator Char *() const;
	Char& String::operator [](UInt32 Pos);
	bool operator < (const String& Str) const;
	bool operator > (const String& Str) const;
	bool operator <=(const String& Str) const;
	bool operator >=(const String& Str) const;
	bool operator ==(const String& Str) const;
	bool operator !=(const String& Str) const;
	bool operator < (const Char *Chr) const;
	bool operator > (const Char *Chr) const;
	bool operator <=(const Char *Chr) const;
	bool operator >=(const Char *Chr) const;
	bool operator ==(const Char *Chr) const;
	bool operator !=(const Char *Chr) const;

	static UInt32 Length(const Char* String);

	friend String CODE_SYMBOL_PORT operator +(const String& Str1, const String& Str2);

protected:
	void Optimize();
	void Grow(UInt32 Num);

protected:
	Char *Text;
	UInt32 Size;
	UInt32 Len;
};


String::String()
{
	Text = NULL;
	Len = Size = 0;
};

String::String(const String& String)
{
	Size = String.Size;
	Len = String.Len;
	if (Size && Len)
	{
		Text = new Char[Size];
		_tcscpy(Text, String.Text);
	}
	else
	{
		Text = NULL;
	};
};

String::String(const Char *String)
{
	if (String == NULL)
	{
		Text = NULL;
		Size = Len = 0;
		return;
	};

	Size = Length(String) + 1;
	Len = Size-1;
	Text = new Char[Size];
	for (UInt32 i=0 ; i<Len ; i++)
		Text = (Char)String;
	Text[Len] = T('\0');
};

//String::String(const Char* Str, UInt32 Start, UInt32 Count)
//{
//	if (!IsValidIndex(Start) || Count <= 0)
//	{
//		Text = T("");
//		return;
//	};

//	Char *Text = new Char[Count + 1];
//	_tcsncpy(Text, &Str[Start], Count);
//	Text[Count] = T('\0');

//	return;
//};

String::~String()
{
	if (Size && Text != NULL)
		delete[] Text;
};

Char *String::GetString() const
{
	return Text;
};
const UInt32 String::GetLength() const
{
	return Len;
};
const bool String::IsNull() const
{
	return (Text == NULL || !Length(Text));
};
const bool String::IsValidIndex(const UInt32 Index) const
{
	return (Index >= 0 && Index < Len);
};

UInt32 String::Compare(const String& String, bool IgnoreCase) const
{
	if (IsNull() && !String.IsNull())
		return 1;
	else if (!IsNull() && String.IsNull())
		return -1;
	else if (IsNull() && String.IsNull())
		return 0;
	/////////????????????????????//////////
	if (IgnoreCase)
		return _tcsicmp(Text, String.Text);
	else
		return _tcscmp(Text, String.Text);
};
bool String::Find(String& Str, UInt32& Pos, bool IgnoreCase) const
{
	if (IsNull() || Str.IsNull())
		return false;

	int (* cmpfn)(const Char*, const Char*, size_t) = (IgnoreCase) ? _tcsnicmp : _tcsncmp;

	for (Pos=0; Pos <= (Len-Str.Len); Pos++)
	{
		if(IgnoreCase)
			if (_tcsnicmp(&Text[Pos], Str.Text, Str.Len) == 0)
				return true;
		else
			if (_tcsncmp(&Text[Pos], Str.Text, Str.Len) == 0)
				return true;
	};

	return false;
};
void String::Delete(UInt32 Pos, UInt32 Count)
{
	if (Pos > Len || Count==0)
		return;
	if (Pos + Count > Len)
		Count = (Len - Pos);

	for (UInt32 i=Pos ; i<Len-Count ; i++)
		Text = Text[i+Count];

	Len -= Count;
	Text[Len] = T('\0');
	Optimize();
};
void String::Insert(UInt32 Pos, Char c)
{
	if (Pos<0 || Pos>Len)
	return;

	Grow(1);

	memmove((void *)&Text[Pos+1], (const void *)&Text[Pos], Len-Pos);
	Text[Pos] = c;
	Text[++Len] = T('\0');
};
void String::Insert(UInt32 Pos, const String& String)
{
	if (Pos<0 || Pos>Len || String.IsNull())
	return;

	Char *New = new Char[String.Len + Len + 1];

	if (Pos > 0)
		_tcsncpy(New, Text, Pos);

	_tcsncpy(&New[Pos], String.Text, String.Len);

	if (Len-Pos > 0)
		_tcsncpy(&New[Pos+String.Len], &Text[Pos], Len-Pos);

	if (New == NULL)
	{
		Text = NULL;
		Size = Len = 0;
		return;
	};

	Size = Length(New) + 1;
	Len = Size-1;
	Text = new Char[Size];
	_tcscpy(Text, New);
};

void String::ClipLeadingWhitespace()
{
	if (IsNull())
		return;
	UInt32 i;
	for (i=0 ; i<Len ; i++)
	{
		if (Text != 0x20 && Text != 0x09)
		break;
	};
	Delete(0, i);
};

void String::ClipTrailingWhitespace()
{
	if (IsNull())
		return;
	UInt32 i;
	for (i=Len-1 ; i>=0 ; i--)
	{
		if (Text != 0x20 && Text != 0x09)
		break;
	};
	Delete(i+1, Len-i-1);
};

void String::Trim()
{
	ClipLeadingWhitespace();
	ClipTrailingWhitespace();
};

UInt32 String::ToInt() const
{
	if (IsNull())
		return 0;
#		if CODE_UNICODE == 1
		return _wtoi(Text);
#		else
		return atoi(Text);
#		endif
};

void String::operator=(const String& str)
{
	Size = str.Size;
	Len = str.Len;
	if (Size && Len)
	{
		Text = new Char[Size];
		_tcscpy(Text, str.Text);
	}
	else
	{
		Text = NULL;
	};
};
void String::operator=(const Char *str)
{
	Len = Length(str);
	Size = Len + 1;
	if (Size && Len)
	{
		Text = new Char[Size];
		_tcscpy(Text, str);
	}
	else
	{
		Text = NULL;
	};
};

void String::operator +=(const String& String)
{
	if (String.Len > 0)
	{
		Grow(String.Len);
		_tcsncpy(&Text[Len], String.Text, String.Len);
		Len += String.Len;
	};
};

String::operator Char *() const
{
	return Text;
};
Char& String::operator [](UInt32 Pos)
{
	if (Pos < 0)
		return Text[0];
	else if (Pos >= Len)
	{
		Grow(Pos+2);
		return Text[Pos];
	}
	else
		return Text[Pos];
};

bool String::operator < (const String& Str) const	{ return (bool)(Compare(Str) == -1);				};
bool String::operator > (const String& Str) const	{ return (bool)(Compare(Str) == 1);					};
bool String::operator <=(const String& Str) const	{ return (bool)(Compare(Str) != 1);					};
bool String::operator >=(const String& Str) const	{ return (bool)(Compare(Str) != -1);				};
bool String::operator ==(const String& Str) const	{ return (bool)(Compare(Str) == 0);					};
bool String::operator !=(const String& Str) const	{ return (bool)(Compare(Str) != 0);					};
bool String::operator < (const Char *Chr) const		{ return (bool)(Compare(String(Chr)) == -1);	};
bool String::operator > (const Char *Chr) const		{ return (bool)(Compare(String(Chr)) == 1);		};
bool String::operator <=(const Char *Chr) const		{ return (bool)(Compare(String(Chr)) != 1);		};
bool String::operator >=(const Char *Chr) const		{ return (bool)(Compare(String(Chr)) != -1);	};
bool String::operator ==(const Char *Chr) const		{ return (bool)(Compare(String(Chr)) == 0);		};
bool String::operator !=(const Char *Chr) const		{ return (bool)(Compare(String(Chr)) != 0);		};

void String::Optimize()
{
	Size = Len + 1;
	Char *Temp = new Char[Size];
	_tcscpy(Temp, Text);
	delete[] Text;
	Text = Temp;
};
void String::Grow(UInt32 Num)
{
	Size += Num;
	Char *Temp = new Char[Size];
	_tcscpy(Temp, Text);
	delete[] Text;
	Text = Temp;
};

UInt32 String::Length(const Char* String)
{
	UInt32 Length = 0;

	while(*String++)
		++Length;

	return Length;
};

String operator +(const String& Str1, const String& Str2)
{
	Char *Temp = new Char[Str1.Size + Str2.Size];
	if (Str1.Len)
		_tcscpy(Temp, Str1.Text);
	if (Str2.Len)
		_tcscpy(&Temp[Str1.Len], Str2.Text);

	String Result = Temp;
	return Result;
};


Thanks for any thoughts[smile], Chad Seibert [Edited by - Chad Seibert on April 6, 2007 3:07:05 PM]

Share this post


Link to post
Share on other sites
Advertisement
What's the difference between Size and Len?

Share this post


Link to post
Share on other sites
I didn't look through the function implementation but if they work thats a nice set of functionality I wish was included in std::string.

From what I understand the std::string implementation uses every advanced templating feature and is something like 3k lines of code. At least thats what I've been told. So i'll bet speed wise your will be a bit worse. But over all pretty nice.

Might wanna add in like a reverseFind() as well.

Would be absolutely rock if you included other data type parsing to string in there. Overload the + operator to parse ints and floats onto it and that'd b freaking awesome.

Maybe a tokenize set of functions?
setTokenizerDelimitor( char c ) // defaults to " "
getNumTokens();
getToken( int i ); // u know the # of tokens available from the above func

Also probably want a toArray() func.

[edit] just saw u have Char *GetString() const; which does that.
Also you might want to stay away from reserved words like delete even if you are capitalizing it, just less confusing. Use erase or remove or something maybe. Just food for thought.[/edit]

But over all pretty cool man!

[edit]
Ha, ya I've gotta strongly agree with the post below mine. It's like defining a word and using that word in the definition or something... Kinda self defeating.
Unless I guess this is just kinda a wrapper to include more advanced functionality onto std:string.
[/edit]

Share this post


Link to post
Share on other sites
Here's a few things off the top of my head.

(1) Find the invariant(s) for your class and ensure they always hold for any valid object of your class. In the case of a string class, one of those things is that the string is always a valid string of length zero or more. That means do not allow the string object to get in a state in which a function like IsNull() has any meaning.

(2) Prvide a minimal interface (as small as nececssary but no smaller). You don't need an IsValidIndex() function if you have a GetLength() function, since the a valid index of a string must be on (0, GetLength()].

(3) Technically you don't need to overload operators on const Char* since you have a converting constructor. I understand providing them as an optimization, but your implementation is just doing what the compiler-generaed functions would do.

(4) Your assignment operator leaks memory. Okay, it pours memory into the bitbucket. Consider providing a swap() nothrow() operator an implementing your assignment operator in terms of the copy constructor and swap instead. Less code to maintain, fewer leaks, improved exception safety.

(5) Your use of UInt32 (a non-standard type, but I'm guessing it's a 32-bit unsigned int type) is limiting. Why not typedef a size_type (it can be UInt32 for now) to make future changes easier? Or, you could take the attitude that nobody will need more than 640K of RAM.

(6) Consider providing a range constructor so you can play nicely with other language featiures, like the standard library.

(7) Use your Grow() member to grow the string, always. You can modify the Grow() function to use various allocaton strategies and immediately benefit.

(8) Use initializer lists in your constructors.

(9) Use the standard library where possible: std::copy() is likely to as efficient or more efficient that hand-rolling loops. It will do the right thing so you don't have to use C casts with ::memmove and you don't have to revert to non-standard non-portable functions like _tcsncpy() and the like. Just because you're writing a non-standard string class doesn't mean you have to avoid the standard library and make it non-portable.

As I said, those are just a few thoughts off the top of my head.

--smw

Share this post


Link to post
Share on other sites
Quote:
Original post by Sneftel
What's the difference between Size and Len?


Size includes the null, len doesn't. Now that you mention it, it does seem to be kind of worthless, though. And I will change Delete to Remove.

Thanks,
Chad Seibert

Share this post


Link to post
Share on other sites
1. You're missing the const version of operator []
2. You've implemented your comparison operators as member functions, instead of two-param friend functions, which prevents comparing a char* on the left hand side to a String oin the right-hand side.
3. Should use initialiser lists in constructor.
4. Destructor does not need to check for NULL before calling delete [].
5. Implicit conversions are very troublesome. best to remove the operator char *() and use GetString.
6. Why noy make the class use TCHARs? You do use the _tcscmp routines etc, and at the moment it will break if unicode is defined. The memmove will need *sizeof(TCHAR) too.
7. Compare preforms a few more IsNull calls than it needs to - could optimise this.
if (IsNull()) return String.IsNull() ? 0 : 1;
if (String.IsNull()) return -1;


Darn I have to go, but I'm sure there's more.

Share this post


Link to post
Share on other sites
Well, I've gotten through 2, 3, 5, 7, and 8 on Bregma's list of things. I still don't quite understand what you mean by "invariants". What do you exactly mean by a range constructor. Just operator[]? And regarding number 9, the objective is to make a portable string class, just to see if I can. And how can I create this swap nothrow operator using my copy constructor?

Thanks for all the suggestions, it's great!

[Edited by - Chad Seibert on April 6, 2007 4:35:20 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Chad Seibert
I still don't quite understand what you mean by "invariants".

The basic rules an instance of the class must follow to be valid. (In this case, it *must* always point to a valid string of length >= 0.
Any string class that can contain an invalid string is hopelessly flawed. (and as said above, what's the point in a function testing if the string is null?)

Quote:
What do you exactly mean by a range constructor.

A constructor that takes a range of characters as argument (typically in the form of start and end iterators)

Share this post


Link to post
Share on other sites
As for iMalc, I've implemented 3, 4, 5, 6. Regarding 7, my Char type is typedef'd to either unsigned char or wchar_t based on whether unicode support is enabled during compilation(_UNICODE). And what do you mean I'm missing the const version of operator[]?

Sorry for all the questions, it's how I learn[smile],
Chad Seibert

Share this post


Link to post
Share on other sites
Quote:
Original post by Chad Seibert
And what do you mean I'm missing the const version of operator[]?


const String foo = "foobar";

char c = foo[0];

What do you expect that to do? foo is a const object, but there is no const version of operator [], so the code above will not compile (trying to call a non-const member function on a const object).

Add a const version of the operator aswell. The code is exactly the same, just a const and a non-const version. Note that you cannot implement one with the other either.

const Char& String::operator [](UInt32 Pos) const
{
...
};

... which leads me to another point. Why do you have semicolons at the end of every function definition? And on a second look, you have a semicolon after every if-statement also. Although legal, it is extremely dangerous. Remove all semicolons that aren't necessary. Additional semicolons aren't useless and can cause trouble.

Share this post


Link to post
Share on other sites
Well, there are a few things I can see that would make things easier. First, how about having a constructor that takes an std::string, to make it easier to integrate with current applications.

Secondly, I would take a look at the C# implementation of string, which is so beautiful that it makes me cry when I work in C++. It has quite a few interesting things that if you implemented for C++ would actually make your lib quite useful.

Third, you don't appear to have a SubString method, which is very useful when doing a lot of string work.

Last, I haven't looked through your implementation, but how well do you handle Unicode, globalization, etc. Although you may just skip all that because it is only an experiment.

Share this post


Link to post
Share on other sites
Quote:
Original post by Chad Seibert
I still don't quite understand what you mean by "invariants".

An invariant is something about an object of your class (for a class invariant, there are other types like function invariants) that must always be true. For example, for your string class, you could choose the invariant that the string must always be of length >= 0. A zero-length string is not the same as a null string. With the variant mentioned above, your string object could never be null.

If you guarantee your class invariants, you never have to check if your objects are valid. That not only simplifies your code, but makes reasoning about objects easier.

If an invariant cannot be obtained (eg. when the object is constructed) or maintained (after the execution of a member function), that's an indication that you need to throw an exception.
Quote:
What do you exactly mean by a range constructor.

A range constructor takes a range as input. Generally, a half-open interval. Most collection classes in the standard library have one. Here's an example.

String::String(const Char* begin, const Char* end)
: Size(end-begin)
{
Grow(0);
std::copy(begin, end, Text);
}

Quote:
And regarding number 9, the objective is to make a portable string class, just to see if I can.

None of the _t*() functions are portable.
Quote:
And how can I create this swap nothrow operator using my copy constructor?


void String::swap(const String& rhs) nothrow()
{
std::swap(Text, rhs.Text);
std::swap(Size, rhs.Size);
}

String& String::operator=(const String& rhs)
{
String tmp(rhs); // create a new copy of rhs
tmp.swap(*this); // swap it with *this
} // destroy the old *this


--smw

Share this post


Link to post
Share on other sites
Thanks a lot for your help! I've implemented everything except the new functions and getting rid of the unportable functions. I've gotten the list down to the following four:

_tcsicmp
_tcscmp
_tcsnicmp
_tcsncmp

I believe that implementing these will be near impossible[sad], though. Maybe there can be some light shed on these though.

Thanks a lot for your help[smile];

Share this post


Link to post
Share on other sites
Quote:

I believe that implementing these will be near impossible[sad], though. Maybe there can be some light shed on these though.

Thanks a lot for your help[smile];


Impossible?

They are just simple comparison and copy algorithms. You even know exactly what they do since you use them for their exact purpose in the code.

They will mostly consist of a single loop, either comparing or copying individual array elements.

Case insensitive comparison is a bit trickier (this is why the std::string class is such a beast), but if you limit yourself to basic ASCII set, it's just a matter of flipping one bit.

Share this post


Link to post
Share on other sites
Ok, so that limits it down to two, however, case-insensitive comparison of unicode strings could get a little bit difficult though.

Share this post


Link to post
Share on other sites
Quote:
Everything everybody else said


Option B: Use std::string.

Incidentally, "operator Char *() const;" allows me to write 'delete String("OMFG this gets deleted twice.");'

Share this post


Link to post
Share on other sites
Quote:
Original post by Deyja
Quote:
Everything everybody else said


Option B: Use std::string.

Incidentally, "operator Char *() const;" allows me to write 'delete String("OMFG this gets deleted twice.");'


Ha ha, very funny. Btw, I got rid of operator Char*().

Edit: Here's the new version of String:


typedef UInt64 StringLength;

class CODE_SYMBOL_PORT String
{
public:
String();
String(const String& String);
String(const Char* String);
//String(const Char* String, StringLength Start, StringLength Count);
virtual ~String();

Char *GetString() const;
const StringLength GetLength() const;

StringLength Compare(const String& Str, Bool IgnoreCase=false) const;
Bool Find(String& Str, StringLength& Pos, Bool IgnoreCase=false) const;
Void Remove(StringLength Pos, StringLength Count);
Void Insert(StringLength Pos, Char c);
Void Insert(StringLength Pos, const String& Str);
Void TrimLeading();
Void TrimTrailing();
Void Trim();
StringLength ToInt() const;

Void operator =(const String& String);
Void operator +=(const String& String);
const Char& operator [](const StringLength Pos);

static StringLength Length(const Char* String);

friend String CODE_SYMBOL_PORT operator+(const String& Str1, const String& Str2);

friend Bool CODE_SYMBOL_PORT operator<(const String& Str1, const String& Str2);
friend Bool CODE_SYMBOL_PORT operator>(const String& Str1, const String& Str2);
friend Bool CODE_SYMBOL_PORT operator<=(const String& Str1, const String& Str2);
friend Bool CODE_SYMBOL_PORT operator>=(const String& Str1, const String& Str2);
friend Bool CODE_SYMBOL_PORT operator==(const String& Str1, const String& Str2);
friend Bool CODE_SYMBOL_PORT operator!=(const String& Str1, const String& Str2);

protected:
Void Optimize();
Void Grow(const StringLength Num);

protected:
Char *Text;
StringLength Size;
StringLength Len;

private:
Void CopyBegString(Char* Desc, const Char* Src, StringLength Len);
StringLength CompareString(const String& Str1, const String& Str2, Bool CaseSensitive) const;
StringLength CompareSubset(const String& Str1, const String& Str2, StringLength Len, Bool CaseSensitive) const;
};




String::String(): Text(NULL), Len(0), Size(0) {}

String::String(const String& Str): Len(Str.Len), Size(Str.Size), Text(NULL)
{
if (Size && Len)
{
Text = new Char[Size];
Text = Str.Text;
}
else
{
Text = NULL;
}
}

String::String(const Char *Str): Text(NULL), Len(0), Size(0)
{
if (Str == NULL)
{
Text = NULL;
Size = Len = 0;
return;
}

Size = Length(Str) + 1;
Len = Size-1;
Text = new Char[Size];
for (StringLength i=0 ; i<Len ; i++)
Text = (Char)Str;
Text[Len] = T('\0');
}

//String::String(const Char* Str, StringLength Start, StringLength Count)
//{
// if (!(Start >= 0 && Start < Len || Count <= 0))
// {
// Text = T("");
// return;
// }

// Char *Text = new Char[Count + 1];
// CopyString(Text, &Str[Start], Count);
// Text[Count] = T('\0');

// return;
//}

String::~String()
{
if (Size && Text != NULL)
delete[] Text;
}

Char *String::GetString() const
{
return Text;
}

const StringLength String::GetLength() const
{
return Len;
}

StringLength String::Compare(const String& Str, Bool IgnoreCase) const
{
if (IgnoreCase)
return CompareString(Text, Str.Text, false);
else
return CompareString(Text, Str.Text, true);
}

Bool String::Find(String& Str, StringLength& Pos, Bool IgnoreCase) const
{
for (Pos=0; Pos <= (Len-Str.Len); Pos++)
{
if(IgnoreCase)
if (CompareSubset(&Text[Pos], Str.Text, Str.Len, false) == 0)
return true;
else
if (CompareSubset(&Text[Pos], Str.Text, Str.Len, true) == 0)
return true;
}

return false;
}

Void String::Remove(StringLength Pos, StringLength Count)
{
if (Pos > Len || Count==0)
return;
if (Pos + Count > Len)
Count = (Len - Pos);

for (StringLength i=Pos ; i<Len-Count ; i++)
Text = Text[i+Count];

Len -= Count;
Text[Len] = T('\0');
Optimize();
}

Void String::Insert(StringLength Pos, Char c)
{
Grow(1);

memmove((Void *)&Text[Pos+1], (const Void *)&Text[Pos], Len-Pos);
Text[Pos] = c;
Text[++Len] = T('\0');
}

Void String::Insert(StringLength Pos, const String& Str)
{
Char *New = new Char[Str.Len + Len + 1];

if (Pos > 0)
CopyBegString(New, Text, Pos);

CopyBegString(&New[Pos], Str.Text, Str.Len);

if (Len-Pos > 0)
CopyBegString(&New[Pos + Str.Len], &Text[Pos], Len-Pos);

if (New == NULL)
{
Text = NULL;
Size = Len = 0;
return;
}

Size = Length(New) + 1;
Len = Size-1;
Text = new Char[Size];
Text = New;
}

Void String::TrimLeading()
{
StringLength i;

for (i=0 ; i<Len ; i++)
{
if (Text != 0x20 && Text != 0x09)
break;
}
Remove(0, i);
}

Void String::TrimTrailing()
{
StringLength i;

for (i=Len-1 ; i>=0 ; i--)
{
if (Text != 0x20 && Text != 0x09)
break;
}
Remove(i+1, Len-i-1);
}

Void String::Trim()
{
TrimLeading();
TrimTrailing();
}

UInt32 String::ToInt() const
{
# if CODE_UNICODE == 1
return _wtoi(Text);
# else
return atoi(Text);
# endif
}

Void String::operator=(const String& str)
{
Size = str.Size;
Len = str.Len;
if (Size && Len)
{
if(Text)
delete[] Text;
Text = new Char[Size];
Text = str.Text;
}
else
{
Text = NULL;
}
}

Void String::operator +=(const String& String)
{
if (String.Len > 0)
{
Grow(String.Len);
CopyBegString(&Text[Len], String.Text, String.Len);
Len += String.Len;
}
}

const Char& String::operator [](const StringLength Pos)
{
if (Pos < 0)
return Text[0];
else if (Pos >= Len)
{
Grow(Pos+2);
return Text[Pos];
}
else
return Text[Pos];
}

Void String::Optimize()
{
Size = Len + 1;
Char *Temp = new Char[Size];
Temp = Text;
delete[] Text;
Text = Temp;
}

Void String::Grow(const StringLength Num)
{
Size += Num;
Char *Temp = new Char[Size];
Temp = Text;
delete[] Text;
Text = Temp;
}

StringLength String::Length(const Char* String)
{
StringLength Length = 0;

while(*String++)
++Length;

return Length;
}

Void String::CopyBegString(Char* Desc, const Char* Src, StringLength Len)
{
for(StringLength i = 0; i < Len; i++)
Desc = Src;

Desc[Len + 1] = T('\0');
}

StringLength String::CompareString(const String& Str1, const String& Str2, Bool CaseSensitive) const
{
if(CaseSensitive)
{
for(StringLength i = 0; i < Str1.Len; i++)
if(Str1 < Str2)
return -1;
else if(Str1 > Str2)
return 1;
else
return 0;
}
else
{

}
}

StringLength String::CompareSubset(const String& Str1, const String& Str2, StringLength Len, Bool CaseSensitive) const
{
if(Len > Str1.Len || Len > Str2.Len)
return -1;

if(CaseSensitive)
{
for(StringLength i = 0; i < Len; i++)
if(Str1 < Str2)
return -1;
else if(Str1 > Str2)
return 1;
else
return 0;
}
else
{
//Good Luck!!!
}
};

String operator +(const String& Str1, const String& Str2)
{
Char *Temp = new Char[Str1.Size + Str2.Size];

if (Str1.Len)
Temp = Str1.Text;
if (Str2.Len)
for(StringLength i = 0; i < 0; i++)
Temp[Str1.Len + i] = Str2.Text;

String Result = Temp;
return Result;
}

Bool operator < (const String& Str1, const String& Str2)
{
return (Bool)(Str1.Compare(Str2) == -1);
}

Bool operator > (const String& Str1, const String& Str2)
{
return (Bool)(Str1.Compare(Str2) == 1);
}

Bool operator <=(const String& Str1, const String& Str2)
{
return (Bool)(Str1.Compare(Str2) != 1);
}

Bool operator >=(const String& Str1, const String& Str2)
{
return (Bool)(Str1.Compare(Str2) != -1);
}

Bool operator ==(const String& Str1, const String& Str2)
{
return (Bool)(Str1.Compare(Str2) == 0);
}

Bool operator !=(const String& Str1, const String& Str2)
{
return (Bool)(Str1.Compare(Str2) != 0);
}



Thanks for all your help!
Chad Seibert

Share this post


Link to post
Share on other sites
Now we're getting to the teenier things.

* Instead of declaring the StringLength typedef outside String, I'd declare it within string, to reduce namespace pollution.

* operator= should return a String&.

* You're still keeping Size and Len as different variables, even though they are trivially related. This is all kinds of a bad idea.

* Why does ToInt return a StringLength?

* Why are you using Void and Char instead of void and char?

Share this post


Link to post
Share on other sites
Do you intentionally want GetString() to return a non-const char*, instead of a const char*? Especially considering that GetString() is a const function. Seems a bit unsafe.

Share this post


Link to post
Share on other sites
You haven't gotten the [] operators right yet:
Quote:
const Char& operator [](const StringLength Pos);

There should be two of them, and they will look like this:
	Char& operator [](StringLength Pos);
const Char& operator [](StringLength Pos) const;
The first one allows you to write:
	String x("abc");
c[1] = 'B';
and the second one allows you to do:

String x("abc");
Char c = x[1];
Without both, you can't do both of these things.

The const you had in the brackets makes no difference, so please yourself with respect to that one.

The return type of operator =, and += should be const String &. This allows the following to work
	String a, b, c;
a = b = c = "abc";

"const" because it prevents the following absurd use:
	(a = b) = c;

Quote:
String::~String()
{
if (Size && Text != NULL) // Remove this line!
delete[] Text;
}
You don't need that "if" statement! It doesn't matter if Text is already NULL, delete will quite safely do nothing. This behaviour is specifically to prevent the need to write an if statement around most calls to delete.

Share this post


Link to post
Share on other sites
Quote:
"const" because it prevents the following absurd use:
	(a = b) = c;


Why? If that's what they want to do, let them do it. Making the return value non-const doesn't cause any behavior that isn't fully expected and reasonable.

Share this post


Link to post
Share on other sites
OK OK, in *this* instance I was being pedantic. It's just that there's no point in modifying an object twice in this manner anyway.

Share this post


Link to post
Share on other sites
Quote:

(a = b) = c;


The built-in types allow one to do just that, so making the return value non-const is just following the accepted C++ philosophy "when in doubt, do as the built-ins do".

Share this post


Link to post
Share on other sites
Quote:
There should be two of them, and they will look like this:

Char& operator [](StringLength Pos);
const Char& operator [](StringLength Pos) const;

The first one allows you to write:

String x("abc");
c[1] = 'B';

and the second one allows you to do:

String x("abc");
Char c = x[1];

Without both, you can't do both of these things.


This is wrong. First, you can do both with the first non-const version. Second, the const version should not return a reference.

Share this post


Link to post
Share on other sites
When I add the const version of operator[], I'm left to wonder what this error means:

cannot convert 'this' pointer from 'const Code::String' to 'Code::String &'



const Char& String::operator [](StringLength Pos) const
{
if (Pos < 0)
return Text[0];
else if (Pos >= Len)
{
Grow(Pos+2);//Error's here
return Text[Pos];
}
else
return Text[Pos];
}




Thanks, and I did change my destructor
Chad Seibert

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
  • Advertisement
  • Popular Tags

  • Advertisement
  • Popular Now

  • Similar Content

    • By Manuel Berger
      Hello fellow devs!
      Once again I started working on an 2D adventure game and right now I'm doing the character-movement/animation. I'm not a big math guy and I was happy about my solution, but soon I realized that it's flawed.
      My player has 5 walking-animations, mirrored for the left side: up, upright, right, downright, down. With the atan2 function I get the angle between player and destination. To get an index from 0 to 4, I divide PI by 5 and see how many times it goes into the player-destination angle.

      In Pseudo-Code:
      angle = atan2(destination.x - player.x, destination.y - player.y) //swapped y and x to get mirrored angle around the y axis
      index = (int) (angle / (PI / 5));
      PlayAnimation(index); //0 = up, 1 = up_right, 2 = right, 3 = down_right, 4 = down

      Besides the fact that when angle is equal to PI it produces an index of 5, this works like a charm. Or at least I thought so at first. When I tested it, I realized that the up and down animation is playing more often than the others, which is pretty logical, since they have double the angle.

      What I'm trying to achieve is something like this, but with equal angles, so that up and down has the same range as all other directions.

      I can't get my head around it. Any suggestions? Is the whole approach doomed?

      Thank you in advance for any input!
       
    • By devbyskc
      Hi Everyone,
      Like most here, I'm a newbie but have been dabbling with game development for a few years. I am currently working full-time overseas and learning the craft in my spare time. It's been a long but highly rewarding adventure. Much of my time has been spent working through tutorials. In all of them, as well as my own attempts at development, I used the audio files supplied by the tutorial author, or obtained from one of the numerous sites online. I am working solo, and will be for a while, so I don't want to get too wrapped up with any one skill set. Regarding audio, the files I've found and used are good for what I was doing at the time. However I would now like to try my hand at customizing the audio more. My game engine of choice is Unity and it has an audio mixer built in that I have experimented with following their tutorials. I have obtained a great book called Game Audio Development with Unity 5.x that I am working through. Half way through the book it introduces using FMOD to supplement the Unity Audio Mixer. Later in the book, the author introduces Reaper (a very popular DAW) as an external program to compose and mix music to be integrated with Unity. I did some research on DAWs and quickly became overwhelmed. Much of what I found was geared toward professional sound engineers and sound designers. I am in no way trying or even thinking about getting to that level. All I want to be able to do is take a music file, and tweak it some to get the sound I want for my game. I've played with Audacity as well, but it didn't seem to fit the bill. So that is why I am looking at a better quality DAW. Since being solo, I am also under a budget contraint. So of all the DAW software out there, I am considering Reaper or Presonus Studio One due to their pricing. My question is, is investing the time to learn about using a DAW to tweak a sound file worth it? Are there any solo developers currently using a DAW as part of their overall workflow? If so, which one? I've also come across Fabric which is a Unity plug-in that enhances the built-in audio mixer. Would that be a better alternative?
      I know this is long, and maybe I haven't communicated well in trying to be brief. But any advice from the gurus/vets would be greatly appreciated. I've leaned so much and had a lot of fun in the process. BTW, I am also a senior citizen (I cut my programming teeth back using punch cards and Structured Basic when it first came out). If anyone needs more clarification of what I am trying to accomplish please let me know.  Thanks in advance for any assistance/advice.
    • By Yosef BenSadon
      Hi , I was considering this start up http://adshir.com/, for investment and i would like a little bit of feedback on what the developers community think about the technology.
      So far what they have is a demo that runs in real time on a Tablet at over 60FPS, it runs locally on the  integrated GPU of the i7 . They have a 20 000 triangles  dinosaur that looks impressive,  better than anything i saw on a mobile device, with reflections and shadows looking very close to what they would look in the real world. They achieved this thanks to a  new algorithm of a rendering technique called Path tracing/Ray tracing, that  is very demanding and so far it is done mostly for static images.
      From what i checked around there is no real option for real time ray tracing (60 FPS on consumer devices). There was imagination technologies that were supposed to release a chip that supports real time ray tracing, but i did not found they had a product in the market or even if the technology is finished as their last demo  i found was with a PC.  The other one is OTOY with their brigade engine that is still not released and if i understand well is more a cloud solution than in hardware solution .
      Would there  be a sizable  interest in the developers community in having such a product as a plug-in for existing game engines?  How important  is Ray tracing to the  future of high end real time graphics?
    • By bryandalo
      Good day,

      I just wanted to share our casual game that is available for android.

      Description: Fight your way from the ravenous plant monster for survival through flips. The rules are simple, drag and release your phone screen. Improve your skills and show it to your friends with the games quirky ranks. Select an array of characters using the orb you acquire throughout the game.

      Download: https://play.google.com/store/apps/details?id=com.HellmodeGames.FlipEscape&hl=en
       
      Trailer: 
       
    • By khawk
      Watch the latest from Unity.
       
  • Advertisement