Basic String class

I wanted to create a basic String class that will hold a CString and have:

• constructor, destructor, copy constructor

• assignment operator (both from String and CString)

• > operators to stream with cout and cin

• ==, ` operators to compare Strings






• [ ] operator to access a certain char in a string






• Len() function to get the length of the string






• CStr() to access the C string in the String class

This is really a part of my baby steps in C++, so any feedback about style, design and coding will be appreciated. Attached are both the header file (the interface, as I learned it is called) and the cpp file:

/*  
    string.h
    Written by me on Jan 27, 2015
*/

#include 

using namespace std;

namespace exercises
{

class String;
ostream& operator>(istream&, String&);

class String
{
public:
    explicit String(const char* str_="");
    ~String();

    String(String const &oStr_);    

    String& operator=(const String& oStr_);
    String& operator=(const char* oStr_);

    bool operator==(const String& oStr_)const;
    bool operator(const String& oStr_)const;   

    char operator[](std::size_t)const;
    char& operator[](std::size_t);  

    const std::size_t Len()const;   
    const char* CStr()const;

private:
    char* m_str;
};

} //namespace exercises

``
/* string.cpp
Written by me on Jan 27, 2015
*/

#include
#include
#include "string.h"

using namespace std;

namespace exercises
{

static int BUFFSIZE = 256; // buffer size limit for streams

// ---------- C~=tor funcs ----------

String::String(const char* str_)
{
m_str = new char[strlen(str_)+1];
strcpy(m_str, str_);
}

String::~String()
{
delete[] m_str;
}

String::String(String const &oStr_)
{
m_str = new char[strlen(oStr_.m_str)+1];
strcpy(m_str, oStr_.m_str);
}

String& String::operator=(const String& oStr_)
{
char* newString = new char[strlen(oStr_.m_str)+1];
delete[] m_str;

m_str = newString;
strcpy(m_str, oStr

Don't use this:

using namespace std;

Your infraction here is even worse than most people.

NEVER EVER EVER put this in a header file. It not only pollutes the namespace for your code but covertly infects the code of anybody that uses your header file (you can potentially break code just because they added your headerfile).

Putting it in your source file is still pretty bad as it makes certain errors hard to spot and some types of error are hidden from you. see: Why is “using namespace std;” considered bad practice?

Prefer to use the copy and swap idiom for assignment operator:

String& String::operator=(String oStr_)        // Note the pass by value generates
{                                              // your copy for you automatically.
    std::swap(m_str, oStr_.m_str);
}

Again for assignemnt from a C-String you can use copy and swap:

String& String::operator=(const char* oCStr_)
{
    String tmp(oCStr_);
    std::swap(m_str, oCStr_.m_str());  // PS I hate that trailing underscore.
}

Your are trying not to tightly couple your output operator with the code (its a nice thought). But input and output are really intrinsically bound to the implementation. If you change the implementation you are going to have to make changes to these operators. So you may as well tightly couple their interface to your class (and thus give them access to your members).

class String
{
public:
..... STUFF
private:
    char* m_str;

    friend ostream& operator<<(ostream& os_, const String& str_)
    {
        return os_ << str_.m_str;
    }
};

Why bother with new and delete in the same method?

char* newCStr = new char[BUFFSIZE];

// STUFF

delete[] newCStr;

Much easier to just delcare a local buffer:

char newCStr[BUFFSIZE];

// STUFF

Now you let scope take care of it. If BUFFSIZE is huge then you way want to use an object that does the memory mangement for you.

std::vector newCStr(BUFFSIZE);

// STUFF

StackExchange Code Review Q#79135, answer score: 5