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.
critique my string class
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
Author
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:
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];
_tcsicmp_tcscmp_tcsnicmp_tcsncmpI 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];
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.
Author
Ok, so that limits it down to two, however, case-insensitive comparison of unicode strings could get a little bit difficult though.
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.");'
Author
Quote:Original post by DeyjaQuote: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
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?
* 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?
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.
You haven't gotten the [] operators right yet:
There should be two of them, and they will look like this:
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
"const" because it prevents the following absurd use:
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; String x("abc"); c[1] = 'B'; String x("abc"); Char c = x[1];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()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.
{
if (Size && Text != NULL) // Remove this line!
delete[] Text;
}
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
