basic_string implementation

I have attempted to make a basic version [not complete] of the std::basic_string type in C++, and I would like to make sure that I have done everything correctly, and efficiently, as I can be prone to leaking memory in my programs.

Note: I also created an allocator class, which works the same as the std equivalent, but I have not added it, as I would like to focus on the implementation of the basic_string (and char_traits).

```
template struct char_traits
{
};

template <> struct char_traits
{
typedef char _Elem;
static std::size_t length(const _Elem *_Str)
{
return strlen(_Str);
}
static int compare(const _Elem _Lhs, const _Elem _Rhs, std::size_t _Count)
{
return strncmp(_Lhs, _Rhs, _Count);
}
};

template <> struct char_traits
{
typedef wchar_t _Elem;
static std::size_t length(const _Elem *_Str)
{
return wcslen(_Str);
}
static int compare(const _Elem _Lhs, const _Elem _Rhs, std::size_t _Count)
{
return wcsncmp(_Lhs, _Rhs, _Count);
}
};

template struct _Char_Traits
{
};

template <> struct _Char_Traits
{
typedef char _Elem;
static int va_printf(_Elem _Dest, const _Elem _Format, va_list _Args)
{
return vsprintf(_Dest, _Format, _Args);
}
};

template <> struct _Char_Traits
{
typedef wchar_t _Elem;
static int va_printf(_Elem _Dest, const _Elem _Format, va_list _Args)
{
return vswprintf(_Dest, _Format, _Args);
}
};

template , typename _Alloc = allocator > class basic_string
{
public:
typedef basic_string _Myt;
typedef _Elem value_type;
typedef _Traits traits_type;
typedef _Alloc allocator_type;
typedef value_type *pointer;
typedef const value_type *const_pointer;
typedef value_type *iterator;
typedef const value_type *const_iterator;
typedef value_type &reference;
typedef const value_type &const_reference;
typedef std::size_t size_type;
typedef std::ptrdiff_t diff

First stop using underscores like that.

• Double underscore is always reserved for the implementation.

• A leading underscore followed by a capital letter is always reserved for the implementation.

There are a couple of more rules. But basically unless you want to memorize stop using identifiers with double or a leading underscore. The problem is most beginners seem to want to learn from the standard library and pick that habit up there. The standard library is part of the implementation and thus they can do it. But you as an application level programmer can not.

Bind the & and * to the type not the variable. They are part of type declaration not part of the variable.

int*     x; // x is the variable  `int*` is the type.

Tidy up your declarations; they are hard to read.

typedef basic_string _Myt;
typedef _Elem value_type;
typedef _Traits traits_type;
typedef _Alloc allocator_type;
typedef value_type *pointer;
typedef const value_type *const_pointer;
typedef value_type *iterator;
typedef const value_type *const_iterator;
typedef value_type &reference;
typedef const value_type &const_reference;
typedef std::size_t size_type;
typedef std::ptrdiff_t difference_type;

Much easier to read:

typedef basic_string _Myt;
typedef Elem               value_type;
typedef Traits             traits_type;
typedef Alloc              allocator_type;
typedef value_type*        pointer;
typedef const value_type*  const_pointer;
typedef value_type*        iterator;
typedef const value_type*  const_iterator;
typedef value_type&        reference;
typedef const value_type&  const_reference;
typedef std::size_t        size_type;
typedef std::ptrdiff_t     difference_type;

Your use if allocator is making the code harder to read than necessary. You should have written the code without allocators. Got it reviewed and corrected then added the allocator where necessary.

_Alloc().construct() // Used to call the constructor
                         // when doing an inplace new.
    _Alloc(). destroy()  // Used to call the destructor on
                         // an object that was created by
                         // inplace new

We are dealing with characters and thus construction and destruction are completely irelavant to a string.

Fundamental flaw in your design is that you don't keep track of the string size as a member of the class. You re-calculate it every time by searching along the string looking for the \0 marker. This is a real problem with your design fix it otherwise your class is little better than a C-String.

Are you really only going to allocate 1 byte in the constructor?

basic_string()
{
    __data = _Alloc().allocate(1);
    _Alloc().construct(&__data[0], '\0');
}

Its likely that string will be expanded real soon. usually you see to size variables in this type of container. The amount currently being used (size). The amount we have allocated for use (reserved).

This is a dasterdly and very dangerious thing to do:

basic_string(void *_Init)
{
    *this = basic_string(reinterpret_cast(_Init));
}

Most pointers auto convert to void* so your string here will initialize with any pointer and pretend it is a string. NOT a good idea. Make people be specific.

Don't need to check for copy construction against yourself.

basic_string(const _Myt &_Init)
{
    if (this != &_Init) {
        ...
    }
}

You are not constructed yet. So you can not be passed as an argument to another constructor so there can never be copy construction from self.

This is a very expensive copy operation.

*this = basic_string(_Init.c_str());

You convert to a C-String. Build a separate object that does not use the intrinsic properties of the class but builds from a native C-String (this is a class I would expect it to know its own size (here you are re-calculating it). Then you perform an assignment (which usually involved creating another copy and destroying it).

It should be as simple as this (add your allocator as needed).

basic_string(Myt const& copy)
     : data(Alloc().allocate(copy.size+1))
     , size(copy.size)
  {
      std::copy(©.data[0], ©.data[copy.size+1], &data[0]);
  }

Everywhere else in your code you use Alloc().construct() but in the assign() you use std::copy().

std::copy(&_Rhs[0], &_Rhs[_Traits::length(_Rhs)], &buf[0]);
    _Alloc().construct(&buf[_Traits::length(_Rhs)], '\0');

Be consistent.

You have defined an assignment operator for C-String but not for real assignment. So you have not fulfilled the rule of three. because you did not define one the compiler generated one for you and it will not work correctly with RAW owned pointers.

Its simple to implement use the copy and swap idum.

Myt &operator=(Myt rhs) // pass by value to get a copy.
{
    rhs.swap(*this);    // Swap the content with the copy.
    return *this;
}

std::size is unsigned it can never be negative.

```
reference at(size_typ

StackExchange Code Review Q#42477, answer score: 6