improving my string implementation

[JLBorges]

> So I believe I have to make a check to see if the buffer is empty? For the illegal accesses.

No.

If the s is empty and the class invariant holds, s.size == 0, and the loop will not be entered into at all.

String::String(const String&s)
{
    size = s.size;
    buffer = new char[size];
    for(int i = 0; i < size; i++){
        buffer[i] = s.buffer[i]; // Q: what memory are you accessing here if s is empty??
        // A: No memory is accessed if s is empty. Control does not reach this line if  size is zero.
    }
}

A simple (and painless) way to implement the assignment operator is:
a. Implement swap
b. Take the rhs of the assignment by value and swap lhs with rhs

For example:

#include <iostream>
#include <cassert>
#include <algorithm>
#include <cstring>

class String {

    std::size_t size ;
    char * buffer ;
    String( std::size_t len ) : size(len), buffer( new char[size]{} ) {}

    public:
        String() : size(0), buffer( new char[0] ) {}

        String( const String& that ) : size(that.size), buffer( new char[size]{} )
        { std::copy( that.buffer, that.buffer+size, buffer ) ; }

        String( const char* cstr ) : size( cstr ? std::strlen(cstr) : 0 ), buffer( new char[size]{} )
        { std::copy( cstr, cstr+size, buffer ) ; }

        ~String() { delete[] buffer ; }

        String( String&& that ) : String() { swap(that) ; } // you may want to ignore this for now

        // simple(st) assignment operator: take rhs by value and swap
        String& operator = ( String that ) { swap(that) ; return *this ; }

        std::size_t length() const { return size ; }

        char& operator[] ( std::size_t pos ) { assert( pos < size ) ; return buffer[pos] ; }
        const char& operator[] ( std::size_t pos ) const { assert( pos < size ) ; return buffer[pos] ; }

        void swap( String& that ) { std::swap(size,that.size) ; std::swap(buffer,that.buffer) ; }

        String& operator += ( const String& that )
        {
            String temp( size + that.size ) ; 
            std::copy( buffer, buffer+size, temp.buffer ) ;
            std::copy( that.buffer, that.buffer+that.size, temp.buffer+size ) ;
            swap(temp) ;
            return *this ;
        }

        friend String operator+ ( String a, const String& b ) { return a += b ; }

        friend bool operator== ( const String& a, const String&b )
        { return std::equal( a.buffer, a.buffer+a.size, b.buffer, b.buffer+b.size ) ; }

        friend bool operator< ( const String& a, const String&b )
        { return std::lexicographical_compare( a.buffer, a.buffer+a.size, b.buffer, b.buffer+b.size ) ; }

        friend bool operator!= ( const String& a, const String& b ) { return !(a==b) ; }
        friend bool operator> ( const String& a, const String&b ) { return b < a ; }
        friend bool operator<= ( const String& a, const String&b ) { return !(a>b) ; }
        friend bool operator>= ( const String& a, const String&b ) { return !(a<b) ; }

        friend std::ostream& operator<< ( std::ostream& stm, const String& s ) { return stm << s.buffer ; }
};

int main()
{
    String s1; // s1 == ""
    assert(s1.length() == 0);

    String s2("hi");  // s2 == "hi"
    assert(s2.length() == 2);

    String s3(s2);  // s3 == "hi"
    assert(s3.length() == 2);

    assert(s3[0] == 'h');
    assert(s3[1] == 'i');

    s1 = s2;  // s1 == "hi"
    assert(s1 == s2);

    s3 = "bye";  // s3 == "bye"
    assert(s3.length() == 3);
    assert(s3[0] == 'b');
    assert(s3[1] == 'y');
    assert(s3[2] == 'e');

    s1 += "re";  // s1 == "hire"
    assert(s1 == "hire");

    s1 += "d"; // s1 == "hired"
    assert(not (s1 == "hire"));

    s1 = std::move(s2) ;
    assert( s1 == "hi" ) ;

    std::cout << "SUCCESS" << '\n' ;
}

http://coliru.stacked-crooked.com/a/787028a2b60c9196

[cppnoob25]

Hey Andy,

How about this?

void String::operator =(const String&s)
{
    if(s.buffer == nullptr)
    {
        delete [] buffer;
        buffer = nullptr;
        size = 0;
    }
    else{
        delete [] buffer;
        size = s.size;
        buffer = new char[size];
        for(int i = 0; i < s.length();i++)
        {
            buffer[i] = s.buffer[i];
        }
    }
}

Although I'm unsure if I should delete the buffer there. And this code still can't handle this code you listed before

String test("Hello");

test = test;

How do I work with code that's referencing itself.

[andywestken]

Well, can you work out how to test if the memory address of the current object is the same as the passed in object?

And what do you need to do if you assign an object to itself?

Andy

PS Once you've got your implementation working you should check out JLBorges' code; it uses what is the standard approach.

[cppnoob25]

Okay I think I got it. Thanks Andy.

I did read through JLBorges code, I just wanted to try coding everything without the use of additional libraries.

Next I'll try the vector class.

[JLBorges]

> I just wanted to try coding everything without the use of additional libraries.

We can't really write any C++ program without an implicit dependency on parts of the language support library. And the input output library presents a nasty problem.

As far as the rest of the standard library is concerned, we can turn out our own home-grown version. The standard library is written using language constructs available to every C++ programmer.

For instance, with just #include <iostream> and a kludged assert mechanism:

#include <iostream>

inline std::size_t strlen( const char* p ) // invariant p != nullptr
{
    std::size_t n = 0 ;
    while( *p++ != 0 ) ++n ;
    return n ;
}

template < typename SRCE_ITERATOR, typename DEST_ITERATOR >
void copy( SRCE_ITERATOR begin, SRCE_ITERATOR end, DEST_ITERATOR dest )
{ while( begin != end ) *dest++ = *begin++ ; }

template < typename ITERATOR_A, typename ITERATOR_B >
bool equal( ITERATOR_A beg_a, ITERATOR_A end_a, ITERATOR_B beg_b, ITERATOR_B end_b )
{
    for( ; beg_a != end_a && beg_b != end_b ; ++beg_a, ++beg_b )
        if( ! ( *beg_a == *beg_b ) ) return false ;
    return beg_a == end_a && beg_b == end_b ; // reached end of both sequences
}

template < typename ITERATOR_A, typename ITERATOR_B >
bool lex_less( ITERATOR_A beg_a, ITERATOR_A end_a, ITERATOR_B beg_b, ITERATOR_B end_b )
{
    for( ; beg_a != end_a && beg_b != end_b ; ++beg_a, ++beg_b )
        if( *beg_a < *beg_b ) return true ;
        else if ( *beg_b < *beg_a ) return false ;
    return beg_a == end_a && beg_b != end_b ; // reached end of a, but did not reach end of b
}

[[ noreturn ]] [[ deprecated( "instead, use the assert in <cassert>" ) ]]
inline bool my_assert_( const char* text, const char* source_file, unsigned int line )
{
   std::cerr <<  "\n\nassertion '" << text << "' failed\n    file: "
             << source_file << " line: " << line << '\n' ;

   // crude workaround to call std::abort() without explicitly including the header
   struct assertion_failure {}; throw assertion_failure {} ;
   // note: without a handler, default behaviour is:
   // std::terminate() => std::terminate_handler => std::abort()
}

#ifdef NDEBUG
    #define my_assert(e) static_cast< const void >(0)
#else
    #define my_assert(e) static_cast< const void >\
        ( ( (!!(e)) || my_assert_( #e, __FILE__, __LINE__ ) ), 0 )
#endif // NDEBUG

class String {

    std::size_t size ;
    char * buffer ;


    public:
        String() : size(0), buffer( new char[0] ) {}

        String( const String& that ) : size(that.size), buffer( new char[size]{} )
        { copy( that.buffer, that.buffer+size, buffer ) ; }

        String( const char* cstr ) : size( strlen(cstr) ), buffer( new char[size]{} )
        { copy( cstr, cstr+size, buffer ) ; }

        // an extra constructor: helper to make operator+= exception safe
        String( std::size_t len ) : size(len), buffer( new char[size]{} ) {}

        ~String() { delete[] buffer ; }

        String( String&& that ) : String() { swap(that) ; } // you may want to ignore this for now

        // simple(st) assignment operator: take rhs by value and swap
        String& operator = ( String that ) { swap(that) ; return *this ; }

        std::size_t length() const { return size ; }

        char& operator[] ( std::size_t pos ) { my_assert( pos < size ) ; return buffer[pos] ; }
        const char& operator[] ( std::size_t pos ) const { my_assert( pos < size ) ; return buffer[pos] ; }

        void swap( String& that )
        {
            const auto s = size ; size = that.size ; that.size = s ;
            const auto b = buffer ; buffer = that.buffer ; that.buffer = b ;
        }

        String& operator += ( const String& that )
        {
            String temp( size + that.size ) ; // makes operator+= exception safe
            copy( buffer, buffer+size, temp.buffer ) ;
            copy( that.buffer, that.buffer+that.size, temp.buffer+size ) ;
            swap(temp) ;
            return *this ;
        }

        friend bool operator== ( const String& a, const String&b )
        { return equal( a.buffer, a.buffer+a.size, b.buffer, b.buffer+b.size ) ; }

        friend bool operator< ( const String& a, const String&b )
        { return lex_less( a.buffer, a.buffer+a.size, b.buffer, b.buffer+b.size ) ; }


        friend std::ostream& operator<< ( std::ostream& stm, const String& s ) { return stm << s.buffer ; }
};

inline String operator+ ( String a, const String& b ) { return a += b ; }
inline bool operator!= ( const String& a, const String& b ) { return !(a==b) ; }
inline bool operator> ( const String& a, const String&b ) { return b < a ; }
inline bool operator<= ( const String& a, const String&b ) { return !(a>b) ; }
inline bool operator>= ( const String& a, const String&b ) { return !(a<b) ; }

int main()
{
    String s1 = "hello" ;
    const String s2 = "world!" ;
    s1 += s2 ;
    const auto s3 = s1 + "!" ;
    my_assert( s3 > s1 ); // print message and abort if assertion fails

    // ...

    std::cout << "\n\n*** result: SUCCESS\n" ;
    
    std::cout << "\ndeliberately trigger an assertion failure: " ;
    my_assert( "this assertion must fail" == nullptr) ;
}

http://coliru.stacked-crooked.com/a/14eb59f2002b95dc

[cppnoob25]

JLBorges your right. Getting to know the libraries is a good thing as well.

Can you tell me how this works?

[[ noreturn ]] [[ deprecated( "instead, use the assert in <cassert>" ) ]]
inline bool my_assert_( const char* text, const char* source_file, unsigned int line )
{
   std::cerr <<  "\n\nassertion '" << text << "' failed\n    file: "
             << source_file << " line: " << line << '\n' ;

   // crude workaround to call std::abort() without explicitly including the header
   struct assertion_failure {}; throw assertion_failure {} ;
   // note: without a handler, default behaviour is:
   // std::terminate() => std::terminate_handler => std::abort()
}

    my_assert( s3 > s1 ); // print message and abort if assertion fails 

your assert method takes in 3 arguments but here I only see one?


I have never seen anything like this either.

[[ noreturn ]] [[ deprecated( "instead, use the assert in <cassert>" ) ]]

And thanks so much for taking the time to code this out completely. As I go through it I really enjoy reading your code and getting myself to understand it.

[JLBorges]

> Getting to know the libraries is a good thing as well.

At least basic familiarity with the standard library is not merely a good thing, it is an essential requirement for any programmer who wants to claim some amount of competence in C++.

In general, use the facilities in the standard library. Writing code that mimics the functionality of parts of the standard library is excellent for purposes of learning C++ - but it remains just that.

Use the library in production code; the advantages are several:
a. A widely used standard library implementation would be more robust and error-free than home-grown code;
a lot of programmers have used it for a number of years in a variety of contexts.
b. The implementors of the standard library know all about their compiler’s innards; the code that they write is likely to be not only correct, but also carefully optimised for that particular implementation.
c. The optimiser knows the standard library and may use that knowledge effectively. For instance, while std::strcpy may be (usually is) replaced by an intrinsic, lounge_kiddie::strcpy is another matter altogether (even if the implementation is visible.)


> Can you tell me how this works?

The first part is straightforward. With:

bool my_assert_( const char* text, const char* source_file, unsigned int line )
{
   std::cerr <<  "\n\nassertion '" << text << "' failed\n    file: "
             << source_file << " line: " << line << '\n' ;
   // ...
}

my_assert_( "vec.empty()", "main.cpp", 24 ) prints out

assertion 'vec.empty()' failed
    file: main.cpp line: 24

The second part struct assertion_failure {}; throw assertion_failure {} ;
is a convoluted way to end the program (subject to the comments in the code).

I was somewhat careless while writing it; this would surely call std::terminate()

[[ noreturn ]] [[ deprecated( "instead, use the assert in <cassert>" ) ]]
inline bool my_assert_( const char* text, const char* source_file, unsigned int line ) noexcept
{
   std::cerr <<  "\n\nassertion '" << text << "' failed: file: "
             << source_file << " line: " << line << '\n' ;

   // crude workaround to call std::abort() without explicitly including the header
   struct assertion_failure { ~assertion_failure() { throw 0 ; } };
   assertion_failure temp ; // making doubly sure; exception thrown during stack unwind: call std::terminate
   throw temp ; // exception thrown from function marked noexcept: call std::terminate
   // note: default behaviour is:
   // std::terminate() => std::terminate_handler => std::abort()
}

See: http://en.cppreference.com/w/cpp/error/terminate


> your assert method takes in 3 arguments but here I only see one?

my_assert() is a preprocessor macro which takes one token as argument.
If NDEBUG is not defined, this macro calls the function my_assert_() with three arguments
(name of the file __FILE__ and line number __LINE__ are added).

...
    #define my_assert(e) static_cast< const void >\
        ( ( (!!(e)) || my_assert_( #e, __FILE__, __LINE__ ) ), 0 ) 

> I have never seen anything like this either.
> [[ noreturn ]] [[ deprecated( "instead, use the assert in <cassert>" ) ]]

Those are two standard C++ attributes. The second one (deprecated) came with C++14.
See: http://en.cppreference.com/w/cpp/language/attributes

[dhayden]

BTW, JLBorges's example demonstrates a bit of fragile code that I always try to avoid:

class String {
    std::size_t size ;
    char * buffer ;
...
    String( const String& that ) : size(that.size), buffer( new char[size]{} )
    { copy( that.buffer, that.buffer+size, buffer ) ; }

This code works just fine, but consider an innocent change. Suppose some OCD manager decides that all class members should be listed in alphabetical order, so size and buffer should be swapped:

class String {
    char * buffer ;
    std::size_t size ;
...

This seems innocent enough, but it actually causes the copy constructor to break because members are initialized in the order in which they are appear in the class definition, NOT the order in which they appear in the initializer list. On other words, the copy constructor now executes buffer(new char[size]) first, followed by size(that.size). As a result, the buffer is sized with the uninitialized size member.

For this reason, I always avoid using a class's members in its own initialization list.

[JLBorges]

If the order of declaration of non-static member objects of a class is changed, its object representation is changed; the sane thing to do is recompile and retest all the code which includes the header directly or indirectly.

Compile with all warnings enabled, and pay attention to the warnings; give the compiler a chance to help you. Compile with more than one compiler; even if one misses out, another can alert you of a possible problem.

struct A
{
    A() : j(1), i(2) {}
    int i ;
    int j ;
};

clang++ -std=c++14 -stdlib=libc++ -O2 -Wall -Wextra -pedantic-errors -c main.cpp
echo ================ && g++ -std=c++14 -O2 -Wall -Wextra -pedantic-errors -c main.cpp
main.cpp:3:11: warning: field 'j' will be initialized after field 'i' [-Wreorder]
    A() : j(1), i(2) {}
          ^
1 warning generated.
================
main.cpp: In constructor 'A::A()':
main.cpp:5:9: warning: 'A::j' will be initialized after [-Wreorder]
     int j ;
         ^
main.cpp:4:9: warning:   'int A::i' [-Wreorder]
     int i ;
         ^
main.cpp:3:5: warning:   when initialized here [-Wreorder]
     A() : j(1), i(2) {}
     ^

http://coliru.stacked-crooked.com/a/b3e653ce013fdf7a

> I always avoid using a class's members in its own initialization list

For member objects with trivial initialisation, a robust approach (if compiler warnings are to be ignored) is the one suggested by Lippman: do not use member initializer lists to initialize them; instead assign to them in the body of the constructor.

String::String( const String& that )
{
     size = that.size ;
     buffer = new char[size]{} ;
     copy( that.buffer, that.buffer+size, buffer ) ; 
}

(This was pretty sound advice at that time; twenty years ago, compilers weren't as helpful or as perceptive as they are today).