patterncppModerate
Implementing a String class using reference count
Viewed 0 times
referenceclassusingcountimplementingstring
Problem
Design a STRING class. Provide support for:
Modify the
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
```
#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
- 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 byanother 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
Free standing Vs member functions
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
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.
Internal Data
Sure the iterator does share the same data type. But it does not have the same meaning. I would have used
Redeclaring functions.
You don't actually need to re-declare the friend functions. The friend declaration in the class should be enough.
noexcept
You should mark the
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
Comparison
I see you are trying to define everything in terms of
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.
DRY Code
Seems like I have seen this clip of code more than once.
May want to wrap this in a function.
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 charSure 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 charvoid 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.