HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Implementing a String class using reference count

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
referenceclassusingcountimplementingstring

Problem

Design a STRING class. Provide support for:



  • Assigning one object for another



  • Two string can be concatenated using + operator



  • Two strings can be compared using the relational operators





Modify the String class so that assigning/initializing a string by
another will not copy it physically but will keep a reference count,
which will be incremented. Reference value 0 means the space can be
released.

I have a pointer to the reference count within each class and keep incrementing it when the string is assigned or copied. The program seems to work. Can this implementation be made simpler, especially the += operator for String?

```
#include
#include
#include
#include
#include

class String {
friend void swap(String &first, String &second);

friend String operator+(String lhs, String const &rhs);

friend bool operator(String const &lhs, String const &rhs);

friend bool operator==(String const &lhs, String const &rhs);
friend bool operator!=(String const &lhs, String const &rhs);

friend std::ostream &operator String(In first, In last);
String(const String &s);

String &operator=(const String &rhs);

size_type size() const;

iterator begin();
const_iterator begin() const;

iterator end();
const_iterator end() const;

String &operator+=(const String &s);

int compare(const String &str) const;

~String();
private:
size_type data_size;
iterator data; // points to the first char
int *ref_cnt;
};

void swap(String &first, String &second);

String operator+(String lhs, String const &rhs);

std::ostream &operator(String const &lhs, String const &rhs);

bool operator==(String const &lhs, String const &rhs);
bool operator!=(String const &lhs, String const &rhs);

// implementation of friend functions

void swap(String &first, String &second)
{
using std::swap;
swap(first.data_size, second.data_size);
swap(first.data, second.data);
swap(first.ref_c

Solution

Overall that's very good.

I did not find any issues with the code. All the comments are very minor.
Design

At a design level there is one change I would make. Note yours works.

But you can put all three members (size/ptr/ref) into a shared object. The advantage to this is that the (size/ptr) are in the sample place. This reduces the chance for them to accidentally go out of sync because of a small error.

There is also the advantage that your String object is reduced in size. From sizeof(ptr) + sizeof(std::size_t) + sizeof(ptr) to simply sizeof(ptr).
Free standing Vs member functions

friend String operator+(String lhs, String const &rhs);

    friend bool operator(String const &lhs, String const &rhs);

    friend bool operator==(String const &lhs, String const &rhs);
    friend bool operator!=(String const &lhs, String const &rhs);


The reason to use a free standing function over a member is to allow either sides of the expression to be auto converted into the class (rather than just the right hand side being potentially auto converted into the class).

Make sure this the expected behavior.
Stealing things from std::string

static const size_t npos = -1;


Not actually used anywhere.
Rule of Three

When I see classes that manage resources; like strings. I want to check that they are obeying the rule of three. I don't immediately see the destructor and I have to scroll down to the bottom of the classes.

String();
    String(size_type count, char c);
    String(const char *cp);
    template String(In first, In last);
    String(const String &s);

    String &operator=(const String &rhs);

    // Lots of other functions.

    ~String();


Internal Data

iterator data; // points to the first char


Sure the iterator does share the same data type. But it does not have the same meaning. I would have used char* to indicate that meaning of the data.
Redeclaring functions.

You don't actually need to re-declare the friend functions. The friend declaration in the class should be enough.

void swap(String &first, String &second);

String operator+(String lhs, String const &rhs);

std::ostream &operator(String const &lhs, String const &rhs);

bool operator==(String const &lhs, String const &rhs);
bool operator!=(String const &lhs, String const &rhs);


noexcept

You should mark the swap() function as no except (it is noexcept) but it makes using it from other noexcept functions useful (like move constructor and move assignment).

void swap(String &first, String &second)
{
    using std::swap;
    swap(first.data_size, second.data_size);
    swap(first.data, second.data);
    swap(first.ref_cnt, second.ref_cnt);
}


Print more efficiently.

This is probably the most inefficient way to print the string. You are calling the output operator for each character in the string. Also the stream operator is already overloaded for char* so you can do this with a single line.

std::ostream &operator<<(std::ostream &os, const String &s)
{
    for (auto c : s)
        os << c;
    return os;
}


Comparison

I see you are trying to define everything in terms of operatorThe constructors allocate the extra byte. But they don't set that last byte to '\0'`. So currently they have indeterminate value.
One line per variable.

Just like declaring one variable per line. It also a good idea to use one line per variable n the initializer list.

String::String():
    data_size(0), data(new char[data_size + 1]()), ref_cnt(new int())

// I would do:

String::String()
    : data_size(0)
    , data(new char[data_size + 1]())
    , ref_cnt(new int())


DRY Code

Seems like I have seen this clip of code more than once.

*ref_cnt = *ref_cnt - 1;
    if (*ref_cnt == 0) {
        delete ref_cnt;
        delete[] data; 
    }


May want to wrap this in a function.

Code Snippets

friend String operator+(String lhs, String const &rhs);

    friend bool operator<(String const &lhs, String const &rhs);
    friend bool operator>(String const &lhs, String const &rhs);

    friend bool operator==(String const &lhs, String const &rhs);
    friend bool operator!=(String const &lhs, String const &rhs);
static const size_t npos = -1;
String();
    String(size_type count, char c);
    String(const char *cp);
    template<class In> String(In first, In last);
    String(const String &s);

    String &operator=(const String &rhs);


    // Lots of other functions.

    ~String();
iterator data; // points to the first char
void swap(String &first, String &second);

String operator+(String lhs, String const &rhs);

std::ostream &operator<<(std::ostream &os, const String &s);

bool operator<(String const &lhs, String const &rhs);
bool operator>(String const &lhs, String const &rhs);

bool operator==(String const &lhs, String const &rhs);
bool operator!=(String const &lhs, String const &rhs);

Context

StackExchange Code Review Q#120691, answer score: 14

Revisions (0)

No revisions yet.