Mixing
C++ implementation of basic_string
$begingroup$
This is a basic_string implementation that has SSO. It's not done, but the fundamental operations are all there (append, erase, resize, reserve, push_back/pop_back). Proper iterators and my own char_traits impl will be added eventually.
#include <string>
template<typename CharType, typename Traits = std::char_traits<CharType>, typename Allocator = std::allocator<CharType>>
class kbasic_string
{
public:
static const size_t npos = -1;
using iterator = CharType*;
using const_iterator = const CharType*;
friend kbasic_string operator+(const kbasic_string& lhs, const kbasic_string& rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, const CharType* rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, CharType rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend std::ostream& operator<<(std::ostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
friend std::wostream& operator<<(std::wostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
kbasic_string() = default;
kbasic_string(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
}
kbasic_string(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
on_heap_ = other.on_heap_;
other.on_heap_ = false;
}
kbasic_string& operator=(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
return *this;
}
kbasic_string& operator=(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
on_heap_ = true;
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
other.on_heap_ = false;
return *this;
}
kbasic_string(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
kbasic_string& operator=(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
void reserve(size_t capacity)
{
if (capacity <= capacity_ || capacity <= 22)
return;
Allocator alloc;
CharType* mem = alloc.allocate(capacity + 1);
if (size_)
std::copy(begin(), end() + 1, mem);
if (on_heap())
alloc.deallocate(data(), capacity_ + 1);
else
on_heap_ = true;
std::memcpy(data_, &mem, sizeof(CharType*));
capacity_ = capacity;
}
void resize(size_t size, CharType app)
{
if (size > size_)
{
reserve(size);
std::fill(end(), end() + (size - size_), app);
*(end() + (size - size_) + 1) = 0;
}
else if (size < size_)
{
erase(end() - (size_ - size), end());
}
else
{
return;
}
size_ = size;
}
void resize(size_t size)
{
resize(size, 0);
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter first, Iter last)
{
return erase(first - begin(), last - first);
}
kbasic_string& erase(size_t pos, size_t count)
{
if (pos > size_)
throw std::out_of_range("pos is out of range");
std::copy(begin() + pos + count, end() + 1, begin() + pos);
size_ -= count;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
{
return erase(it - begin(), 1);
}
void pop_back()
{
erase(end() - 1);
}
void clear()
{
erase(begin(), end());
}
kbasic_string& append(const CharType* str, size_t len)
{
if (size_ + len > capacity_)
reserve(size_ + len);
std::copy(str, str + len + 1, begin() + size_);
size_ += len;
return *this;
}
kbasic_string& append(const CharType* str)
{
return append(str, Traits::length(str));
}
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
kbasic_string& append(CharType ch)
{
if (size_ + 1 > capacity_)
reserve(size_ + 1);
iterator prev_end = begin() + size_;
*prev_end = ch;
*(prev_end + 1) = 0;
++size_;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
void push_back(CharType ch)
{
append(ch);
}
CharType* data() noexcept
{
return on_heap() ? heap_ptr() : data_;
}
const CharType* data() const noexcept
{
return on_heap() ? heap_ptr() : data_;
}
size_t size() const noexcept
{
return size_;
}
size_t length() const noexcept
{
return size_;
}
size_t capacity() const noexcept
{
return capacity_;
}
iterator begin() noexcept
{
return data();
}
const_iterator begin() const noexcept
{
return data();
}
iterator end() noexcept
{
return data() + size_;
}
const_iterator end() const noexcept
{
return data() + size_;
}
bool empty() const noexcept
{
return !size_;
}
CharType& at(size_t n)
{
return data()[n];
}
const CharType& at(size_t n) const
{
return data()[n];
}
CharType& operator(size_t n)
{
return at(n);
}
const CharType& operator(size_t n) const
{
return at(n);
}
kbasic_string& operator+=(const kbasic_string& other)
{
return append(other);
}
kbasic_string& operator+=(const CharType* str)
{
return append(str);
}
kbasic_string& operator+=(CharType ch)
{
return append(ch);
}
bool operator==(const kbasic_string& other)
{
return std::equal(begin(), end(), other.begin(), other.end());
}
bool operator==(const CharType* str)
{
return std::equal(begin(), end(), str, str + Traits::length(str));
}
bool operator==(CharType ch)
{
return size_ == 1 && *begin() == ch;
}
bool operator!=(const kbasic_string& other)
{
return !(*this == other);
}
bool operator!=(const CharType* str)
{
return !(*this == str);
}
bool operator!=(CharType ch)
{
return !(*this == ch);
}
~kbasic_string()
{
if (on_heap())
Allocator{}.deallocate(data(), capacity_ + 1);
}
private:
bool on_heap() const
{
return on_heap_;
}
const CharType* heap_ptr() const
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
CharType* heap_ptr()
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
size_t size_ = 0;
size_t capacity_ = 22;
bool on_heap_ = false;
CharType data_[23] = {0};
};
c++ strings
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
asked Jan 15 at 3:07
Krystian SKrystian S
1211
$endgroup$
add a comment |
$begingroup$
This is a basic_string implementation that has SSO. It's not done, but the fundamental operations are all there (append, erase, resize, reserve, push_back/pop_back). Proper iterators and my own char_traits impl will be added eventually.
#include <string>
template<typename CharType, typename Traits = std::char_traits<CharType>, typename Allocator = std::allocator<CharType>>
class kbasic_string
{
public:
static const size_t npos = -1;
using iterator = CharType*;
using const_iterator = const CharType*;
friend kbasic_string operator+(const kbasic_string& lhs, const kbasic_string& rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, const CharType* rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, CharType rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend std::ostream& operator<<(std::ostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
friend std::wostream& operator<<(std::wostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
kbasic_string() = default;
kbasic_string(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
}
kbasic_string(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
on_heap_ = other.on_heap_;
other.on_heap_ = false;
}
kbasic_string& operator=(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
return *this;
}
kbasic_string& operator=(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
on_heap_ = true;
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
other.on_heap_ = false;
return *this;
}
kbasic_string(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
kbasic_string& operator=(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
void reserve(size_t capacity)
{
if (capacity <= capacity_ || capacity <= 22)
return;
Allocator alloc;
CharType* mem = alloc.allocate(capacity + 1);
if (size_)
std::copy(begin(), end() + 1, mem);
if (on_heap())
alloc.deallocate(data(), capacity_ + 1);
else
on_heap_ = true;
std::memcpy(data_, &mem, sizeof(CharType*));
capacity_ = capacity;
}
void resize(size_t size, CharType app)
{
if (size > size_)
{
reserve(size);
std::fill(end(), end() + (size - size_), app);
*(end() + (size - size_) + 1) = 0;
}
else if (size < size_)
{
erase(end() - (size_ - size), end());
}
else
{
return;
}
size_ = size;
}
void resize(size_t size)
{
resize(size, 0);
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter first, Iter last)
{
return erase(first - begin(), last - first);
}
kbasic_string& erase(size_t pos, size_t count)
{
if (pos > size_)
throw std::out_of_range("pos is out of range");
std::copy(begin() + pos + count, end() + 1, begin() + pos);
size_ -= count;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
{
return erase(it - begin(), 1);
}
void pop_back()
{
erase(end() - 1);
}
void clear()
{
erase(begin(), end());
}
kbasic_string& append(const CharType* str, size_t len)
{
if (size_ + len > capacity_)
reserve(size_ + len);
std::copy(str, str + len + 1, begin() + size_);
size_ += len;
return *this;
}
kbasic_string& append(const CharType* str)
{
return append(str, Traits::length(str));
}
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
kbasic_string& append(CharType ch)
{
if (size_ + 1 > capacity_)
reserve(size_ + 1);
iterator prev_end = begin() + size_;
*prev_end = ch;
*(prev_end + 1) = 0;
++size_;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
void push_back(CharType ch)
{
append(ch);
}
CharType* data() noexcept
{
return on_heap() ? heap_ptr() : data_;
}
const CharType* data() const noexcept
{
return on_heap() ? heap_ptr() : data_;
}
size_t size() const noexcept
{
return size_;
}
size_t length() const noexcept
{
return size_;
}
size_t capacity() const noexcept
{
return capacity_;
}
iterator begin() noexcept
{
return data();
}
const_iterator begin() const noexcept
{
return data();
}
iterator end() noexcept
{
return data() + size_;
}
const_iterator end() const noexcept
{
return data() + size_;
}
bool empty() const noexcept
{
return !size_;
}
CharType& at(size_t n)
{
return data()[n];
}
const CharType& at(size_t n) const
{
return data()[n];
}
CharType& operator(size_t n)
{
return at(n);
}
const CharType& operator(size_t n) const
{
return at(n);
}
kbasic_string& operator+=(const kbasic_string& other)
{
return append(other);
}
kbasic_string& operator+=(const CharType* str)
{
return append(str);
}
kbasic_string& operator+=(CharType ch)
{
return append(ch);
}
bool operator==(const kbasic_string& other)
{
return std::equal(begin(), end(), other.begin(), other.end());
}
bool operator==(const CharType* str)
{
return std::equal(begin(), end(), str, str + Traits::length(str));
}
bool operator==(CharType ch)
{
return size_ == 1 && *begin() == ch;
}
bool operator!=(const kbasic_string& other)
{
return !(*this == other);
}
bool operator!=(const CharType* str)
{
return !(*this == str);
}
bool operator!=(CharType ch)
{
return !(*this == ch);
}
~kbasic_string()
{
if (on_heap())
Allocator{}.deallocate(data(), capacity_ + 1);
}
private:
bool on_heap() const
{
return on_heap_;
}
const CharType* heap_ptr() const
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
CharType* heap_ptr()
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
size_t size_ = 0;
size_t capacity_ = 22;
bool on_heap_ = false;
CharType data_[23] = {0};
};
c++ strings
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
asked Jan 15 at 3:07
Krystian SKrystian S
1211
$endgroup$
$begingroup$
a small thing: since is c++ it should be std::size_t not size_t
$endgroup$
– Sandro4912
Jan 15 at 20:45
add a comment |
$begingroup$
This is a basic_string implementation that has SSO. It's not done, but the fundamental operations are all there (append, erase, resize, reserve, push_back/pop_back). Proper iterators and my own char_traits impl will be added eventually.
#include <string>
template<typename CharType, typename Traits = std::char_traits<CharType>, typename Allocator = std::allocator<CharType>>
class kbasic_string
{
public:
static const size_t npos = -1;
using iterator = CharType*;
using const_iterator = const CharType*;
friend kbasic_string operator+(const kbasic_string& lhs, const kbasic_string& rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, const CharType* rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, CharType rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend std::ostream& operator<<(std::ostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
friend std::wostream& operator<<(std::wostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
kbasic_string() = default;
kbasic_string(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
}
kbasic_string(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
on_heap_ = other.on_heap_;
other.on_heap_ = false;
}
kbasic_string& operator=(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
return *this;
}
kbasic_string& operator=(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
on_heap_ = true;
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
other.on_heap_ = false;
return *this;
}
kbasic_string(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
kbasic_string& operator=(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
void reserve(size_t capacity)
{
if (capacity <= capacity_ || capacity <= 22)
return;
Allocator alloc;
CharType* mem = alloc.allocate(capacity + 1);
if (size_)
std::copy(begin(), end() + 1, mem);
if (on_heap())
alloc.deallocate(data(), capacity_ + 1);
else
on_heap_ = true;
std::memcpy(data_, &mem, sizeof(CharType*));
capacity_ = capacity;
}
void resize(size_t size, CharType app)
{
if (size > size_)
{
reserve(size);
std::fill(end(), end() + (size - size_), app);
*(end() + (size - size_) + 1) = 0;
}
else if (size < size_)
{
erase(end() - (size_ - size), end());
}
else
{
return;
}
size_ = size;
}
void resize(size_t size)
{
resize(size, 0);
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter first, Iter last)
{
return erase(first - begin(), last - first);
}
kbasic_string& erase(size_t pos, size_t count)
{
if (pos > size_)
throw std::out_of_range("pos is out of range");
std::copy(begin() + pos + count, end() + 1, begin() + pos);
size_ -= count;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
{
return erase(it - begin(), 1);
}
void pop_back()
{
erase(end() - 1);
}
void clear()
{
erase(begin(), end());
}
kbasic_string& append(const CharType* str, size_t len)
{
if (size_ + len > capacity_)
reserve(size_ + len);
std::copy(str, str + len + 1, begin() + size_);
size_ += len;
return *this;
}
kbasic_string& append(const CharType* str)
{
return append(str, Traits::length(str));
}
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
kbasic_string& append(CharType ch)
{
if (size_ + 1 > capacity_)
reserve(size_ + 1);
iterator prev_end = begin() + size_;
*prev_end = ch;
*(prev_end + 1) = 0;
++size_;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
void push_back(CharType ch)
{
append(ch);
}
CharType* data() noexcept
{
return on_heap() ? heap_ptr() : data_;
}
const CharType* data() const noexcept
{
return on_heap() ? heap_ptr() : data_;
}
size_t size() const noexcept
{
return size_;
}
size_t length() const noexcept
{
return size_;
}
size_t capacity() const noexcept
{
return capacity_;
}
iterator begin() noexcept
{
return data();
}
const_iterator begin() const noexcept
{
return data();
}
iterator end() noexcept
{
return data() + size_;
}
const_iterator end() const noexcept
{
return data() + size_;
}
bool empty() const noexcept
{
return !size_;
}
CharType& at(size_t n)
{
return data()[n];
}
const CharType& at(size_t n) const
{
return data()[n];
}
CharType& operator(size_t n)
{
return at(n);
}
const CharType& operator(size_t n) const
{
return at(n);
}
kbasic_string& operator+=(const kbasic_string& other)
{
return append(other);
}
kbasic_string& operator+=(const CharType* str)
{
return append(str);
}
kbasic_string& operator+=(CharType ch)
{
return append(ch);
}
bool operator==(const kbasic_string& other)
{
return std::equal(begin(), end(), other.begin(), other.end());
}
bool operator==(const CharType* str)
{
return std::equal(begin(), end(), str, str + Traits::length(str));
}
bool operator==(CharType ch)
{
return size_ == 1 && *begin() == ch;
}
bool operator!=(const kbasic_string& other)
{
return !(*this == other);
}
bool operator!=(const CharType* str)
{
return !(*this == str);
}
bool operator!=(CharType ch)
{
return !(*this == ch);
}
~kbasic_string()
{
if (on_heap())
Allocator{}.deallocate(data(), capacity_ + 1);
}
private:
bool on_heap() const
{
return on_heap_;
}
const CharType* heap_ptr() const
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
CharType* heap_ptr()
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
size_t size_ = 0;
size_t capacity_ = 22;
bool on_heap_ = false;
CharType data_[23] = {0};
};
c++ strings
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
asked Jan 15 at 3:07
Krystian SKrystian S
1211
$endgroup$
This is a basic_string implementation that has SSO. It's not done, but the fundamental operations are all there (append, erase, resize, reserve, push_back/pop_back). Proper iterators and my own char_traits impl will be added eventually.
#include <string>
template<typename CharType, typename Traits = std::char_traits<CharType>, typename Allocator = std::allocator<CharType>>
class kbasic_string
{
public:
static const size_t npos = -1;
using iterator = CharType*;
using const_iterator = const CharType*;
friend kbasic_string operator+(const kbasic_string& lhs, const kbasic_string& rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, const CharType* rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend kbasic_string operator+(const kbasic_string& lhs, CharType rhs)
{
return kbasic_string{lhs}.append(rhs);
}
friend std::ostream& operator<<(std::ostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
friend std::wostream& operator<<(std::wostream& lhs, const kbasic_string& rhs)
{
lhs.write(rhs.data(), rhs.size());
return lhs;
}
kbasic_string() = default;
kbasic_string(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
}
kbasic_string(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
on_heap_ = other.on_heap_;
other.on_heap_ = false;
}
kbasic_string& operator=(const kbasic_string& other)
{
reserve(other.capacity_);
if (other.size_)
std::copy(other.begin(), other.end() + 1, begin());
size_ = other.size_;
return *this;
}
kbasic_string& operator=(kbasic_string&& other)
{
if (other.on_heap())
{
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
on_heap_ = true;
}
else
{
std::copy(other.begin(), other.end() + 1, begin());
}
size_ = other.size_;
capacity_ = other.capacity_;
other.on_heap_ = false;
return *this;
}
kbasic_string(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
kbasic_string& operator=(const CharType* str)
{
size_t size = Traits::length(str);
if (size > capacity_)
reserve(size);
std::copy(str, str + size + 1, data());
size_ = size;
}
void reserve(size_t capacity)
{
if (capacity <= capacity_ || capacity <= 22)
return;
Allocator alloc;
CharType* mem = alloc.allocate(capacity + 1);
if (size_)
std::copy(begin(), end() + 1, mem);
if (on_heap())
alloc.deallocate(data(), capacity_ + 1);
else
on_heap_ = true;
std::memcpy(data_, &mem, sizeof(CharType*));
capacity_ = capacity;
}
void resize(size_t size, CharType app)
{
if (size > size_)
{
reserve(size);
std::fill(end(), end() + (size - size_), app);
*(end() + (size - size_) + 1) = 0;
}
else if (size < size_)
{
erase(end() - (size_ - size), end());
}
else
{
return;
}
size_ = size;
}
void resize(size_t size)
{
resize(size, 0);
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter first, Iter last)
{
return erase(first - begin(), last - first);
}
kbasic_string& erase(size_t pos, size_t count)
{
if (pos > size_)
throw std::out_of_range("pos is out of range");
std::copy(begin() + pos + count, end() + 1, begin() + pos);
size_ -= count;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
{
return erase(it - begin(), 1);
}
void pop_back()
{
erase(end() - 1);
}
void clear()
{
erase(begin(), end());
}
kbasic_string& append(const CharType* str, size_t len)
{
if (size_ + len > capacity_)
reserve(size_ + len);
std::copy(str, str + len + 1, begin() + size_);
size_ += len;
return *this;
}
kbasic_string& append(const CharType* str)
{
return append(str, Traits::length(str));
}
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
kbasic_string& append(CharType ch)
{
if (size_ + 1 > capacity_)
reserve(size_ + 1);
iterator prev_end = begin() + size_;
*prev_end = ch;
*(prev_end + 1) = 0;
++size_;
return *this;
}
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
void push_back(CharType ch)
{
append(ch);
}
CharType* data() noexcept
{
return on_heap() ? heap_ptr() : data_;
}
const CharType* data() const noexcept
{
return on_heap() ? heap_ptr() : data_;
}
size_t size() const noexcept
{
return size_;
}
size_t length() const noexcept
{
return size_;
}
size_t capacity() const noexcept
{
return capacity_;
}
iterator begin() noexcept
{
return data();
}
const_iterator begin() const noexcept
{
return data();
}
iterator end() noexcept
{
return data() + size_;
}
const_iterator end() const noexcept
{
return data() + size_;
}
bool empty() const noexcept
{
return !size_;
}
CharType& at(size_t n)
{
return data()[n];
}
const CharType& at(size_t n) const
{
return data()[n];
}
CharType& operator(size_t n)
{
return at(n);
}
const CharType& operator(size_t n) const
{
return at(n);
}
kbasic_string& operator+=(const kbasic_string& other)
{
return append(other);
}
kbasic_string& operator+=(const CharType* str)
{
return append(str);
}
kbasic_string& operator+=(CharType ch)
{
return append(ch);
}
bool operator==(const kbasic_string& other)
{
return std::equal(begin(), end(), other.begin(), other.end());
}
bool operator==(const CharType* str)
{
return std::equal(begin(), end(), str, str + Traits::length(str));
}
bool operator==(CharType ch)
{
return size_ == 1 && *begin() == ch;
}
bool operator!=(const kbasic_string& other)
{
return !(*this == other);
}
bool operator!=(const CharType* str)
{
return !(*this == str);
}
bool operator!=(CharType ch)
{
return !(*this == ch);
}
~kbasic_string()
{
if (on_heap())
Allocator{}.deallocate(data(), capacity_ + 1);
}
private:
bool on_heap() const
{
return on_heap_;
}
const CharType* heap_ptr() const
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
CharType* heap_ptr()
{
CharType* ptr = nullptr;
std::memcpy(&ptr, data_, sizeof(CharType*));
return ptr;
}
size_t size_ = 0;
size_t capacity_ = 22;
bool on_heap_ = false;
CharType data_[23] = {0};
};
c++ strings
c++ strings
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
asked Jan 15 at 3:07
Krystian SKrystian S
1211
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
asked Jan 15 at 3:07
Krystian SKrystian S
1211
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
edited Jan 15 at 3:16
Jamal♦
30.3k11119227
30.3k11119227
asked Jan 15 at 3:07
Krystian SKrystian S
1211
asked Jan 15 at 3:07
Krystian SKrystian S
1211
asked Jan 15 at 3:07
Krystian SKrystian S
1211
1211
$begingroup$
a small thing: since is c++ it should be std::size_t not size_t
$endgroup$
– Sandro4912
Jan 15 at 20:45
add a comment |
$begingroup$
a small thing: since is c++ it should be std::size_t not size_t
$endgroup$
– Sandro4912
Jan 15 at 20:45
$begingroup$
a small thing: since is c++ it should be std::size_t not size_t
$endgroup$
– Sandro4912
Jan 15 at 20:45
$begingroup$
a small thing: since is c++ it should be std::size_t not size_t
$endgroup$
– Sandro4912
Jan 15 at 20:45
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
It's tricky to review something like this where you're imitating an overcomplicated design, so you know certain things are unimplemented (such as iterators), but then other things might also be unimplemented by accident. For example:
You take a parameter
Traits = std::char_traits<CharType>but then you don't use it! The realbasic_stringwould use the traits class for things like where you usestd::copy.You take an allocator parameter
Allocator = std::allocator<CharType>and do use it, kind of, but you don't go throughstd::allocator_traits<Allocator>. Since all you use isa.allocate()anda.deallocate(), this is probably fine in all real-world situations; but really you should be usingallocator_traits. It can't hurt.You don't store an
Allocatordata member ofkbasic_string; instead, you default-construct one ex nihilo every time you want to use the allocator. This (plus the previous bullet point, technically) means that your container is not allocator-aware: it won't work with user-defined allocators or allocators such asstd::pmr::polymorphic_allocator<char>. You get away with it in your tests right now only becausestd::allocator<char>is (A) empty and (B) default-constructible.You do have tests, right?
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
This is a very strange way of writing
data_ = other.data_;
other.data_ = nullptr;
...oh, I see, your data_ is a char[23], not a char *. Well, okay. I would strongly recommend making it a union of char[23] and char * so that you can copy the pointer member without weird type-punning tricks.
Pedantic nit: Setting all-bits-zero is not technically the same thing as setting to nullptr — the in-memory representation of nullptr need not be all-bits-zero (even though, in practice, it will be).
Also, notice that to be allocator-aware you will have to store a pointer of type std::allocator_traits<Allocator>::pointer, which you definitely cannot assume is all-bits-zero when it's null! For example, boost::interprocess::offset_ptr<char> is not all-bits-zero when it's null.
bool operator==(const kbasic_string& other)
Here and throughout, you forgot the trailing const. These days I recommend making every operator an "ADL friend" non-member:
friend bool operator==(const kbasic_string& other, const kbasic_string& other) {
return std::equal(begin(), end(), other.begin(), other.end());
}
Then it's really obvious if you forget one of the two consts.
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
Write a test case for this function! For example, string("123456789").append(a, a+3). There's a bug in the if condition that leads quickly to undefined behavior.
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
Also write tests for this function! Since str knows its own size(), does it make sense to throw away that information here? What happens if str contains an embedded '' character — what is string("abc").append(string("de", 3))?
Your use of std::_Is_iterator_v<Iter> is sketchy. I would recommend writing your own portable type-trait, something like this:
template<class, class = void> struct is_iterator : std::false_type {};
template<class T> struct is_iterator<T, decltype(void(std::iterator_traits<T>::iterator_category{}))> : std::true_type {};
Orthogonally, you seem to be using _Is_iterator_v almost like pseudocode that magically does whatever you want done. Here you use it to mean "is an input iterator or better":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
Here you use it to mean "is either kbasic_string::iterator or kbasic_string::const_iterator":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
erase should certainly be rewritten as a non-template:
kbasic_string& erase(const_iterator it)
Finally, consider whether you could save space by replacing every instance of on_heap_ with the condition (capacity_ > 22). And consider moving the magic number 22 into a variable!
static constexpr size_t SBO_CAPACITY = 22;
size_t size_ = 0;
size_t capacity_ = SBO_CAPACITY;
CharType data_[SBO_CAPACITY + 1] = {0};
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
$endgroup$
add a comment |
$begingroup$
I was looking at a method which I considered to be really easy: c_str, however, it doesn't look that easy based on how you implemented it.
Currently, you have a bool indicating whether you should store it on the heap or not. If you replace that by a pointer to the first character. You could implement the onHeap by comparing 2 pointers. Every access to the characters can simply use the pointer, unless for the appending.
answered Jan 15 at 7:19
JVApenJVApen
690213
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f211515%2fc-implementation-of-basic-string%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
It's tricky to review something like this where you're imitating an overcomplicated design, so you know certain things are unimplemented (such as iterators), but then other things might also be unimplemented by accident. For example:
You take a parameter
Traits = std::char_traits<CharType>but then you don't use it! The realbasic_stringwould use the traits class for things like where you usestd::copy.You take an allocator parameter
Allocator = std::allocator<CharType>and do use it, kind of, but you don't go throughstd::allocator_traits<Allocator>. Since all you use isa.allocate()anda.deallocate(), this is probably fine in all real-world situations; but really you should be usingallocator_traits. It can't hurt.You don't store an
Allocatordata member ofkbasic_string; instead, you default-construct one ex nihilo every time you want to use the allocator. This (plus the previous bullet point, technically) means that your container is not allocator-aware: it won't work with user-defined allocators or allocators such asstd::pmr::polymorphic_allocator<char>. You get away with it in your tests right now only becausestd::allocator<char>is (A) empty and (B) default-constructible.You do have tests, right?
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
This is a very strange way of writing
data_ = other.data_;
other.data_ = nullptr;
...oh, I see, your data_ is a char[23], not a char *. Well, okay. I would strongly recommend making it a union of char[23] and char * so that you can copy the pointer member without weird type-punning tricks.
Pedantic nit: Setting all-bits-zero is not technically the same thing as setting to nullptr — the in-memory representation of nullptr need not be all-bits-zero (even though, in practice, it will be).
Also, notice that to be allocator-aware you will have to store a pointer of type std::allocator_traits<Allocator>::pointer, which you definitely cannot assume is all-bits-zero when it's null! For example, boost::interprocess::offset_ptr<char> is not all-bits-zero when it's null.
bool operator==(const kbasic_string& other)
Here and throughout, you forgot the trailing const. These days I recommend making every operator an "ADL friend" non-member:
friend bool operator==(const kbasic_string& other, const kbasic_string& other) {
return std::equal(begin(), end(), other.begin(), other.end());
}
Then it's really obvious if you forget one of the two consts.
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
Write a test case for this function! For example, string("123456789").append(a, a+3). There's a bug in the if condition that leads quickly to undefined behavior.
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
Also write tests for this function! Since str knows its own size(), does it make sense to throw away that information here? What happens if str contains an embedded '' character — what is string("abc").append(string("de", 3))?
Your use of std::_Is_iterator_v<Iter> is sketchy. I would recommend writing your own portable type-trait, something like this:
template<class, class = void> struct is_iterator : std::false_type {};
template<class T> struct is_iterator<T, decltype(void(std::iterator_traits<T>::iterator_category{}))> : std::true_type {};
Orthogonally, you seem to be using _Is_iterator_v almost like pseudocode that magically does whatever you want done. Here you use it to mean "is an input iterator or better":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
Here you use it to mean "is either kbasic_string::iterator or kbasic_string::const_iterator":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
erase should certainly be rewritten as a non-template:
kbasic_string& erase(const_iterator it)
Finally, consider whether you could save space by replacing every instance of on_heap_ with the condition (capacity_ > 22). And consider moving the magic number 22 into a variable!
static constexpr size_t SBO_CAPACITY = 22;
size_t size_ = 0;
size_t capacity_ = SBO_CAPACITY;
CharType data_[SBO_CAPACITY + 1] = {0};
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
$endgroup$
add a comment |
$begingroup$
It's tricky to review something like this where you're imitating an overcomplicated design, so you know certain things are unimplemented (such as iterators), but then other things might also be unimplemented by accident. For example:
You take a parameter
Traits = std::char_traits<CharType>but then you don't use it! The realbasic_stringwould use the traits class for things like where you usestd::copy.You take an allocator parameter
Allocator = std::allocator<CharType>and do use it, kind of, but you don't go throughstd::allocator_traits<Allocator>. Since all you use isa.allocate()anda.deallocate(), this is probably fine in all real-world situations; but really you should be usingallocator_traits. It can't hurt.You don't store an
Allocatordata member ofkbasic_string; instead, you default-construct one ex nihilo every time you want to use the allocator. This (plus the previous bullet point, technically) means that your container is not allocator-aware: it won't work with user-defined allocators or allocators such asstd::pmr::polymorphic_allocator<char>. You get away with it in your tests right now only becausestd::allocator<char>is (A) empty and (B) default-constructible.You do have tests, right?
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
This is a very strange way of writing
data_ = other.data_;
other.data_ = nullptr;
...oh, I see, your data_ is a char[23], not a char *. Well, okay. I would strongly recommend making it a union of char[23] and char * so that you can copy the pointer member without weird type-punning tricks.
Pedantic nit: Setting all-bits-zero is not technically the same thing as setting to nullptr — the in-memory representation of nullptr need not be all-bits-zero (even though, in practice, it will be).
Also, notice that to be allocator-aware you will have to store a pointer of type std::allocator_traits<Allocator>::pointer, which you definitely cannot assume is all-bits-zero when it's null! For example, boost::interprocess::offset_ptr<char> is not all-bits-zero when it's null.
bool operator==(const kbasic_string& other)
Here and throughout, you forgot the trailing const. These days I recommend making every operator an "ADL friend" non-member:
friend bool operator==(const kbasic_string& other, const kbasic_string& other) {
return std::equal(begin(), end(), other.begin(), other.end());
}
Then it's really obvious if you forget one of the two consts.
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
Write a test case for this function! For example, string("123456789").append(a, a+3). There's a bug in the if condition that leads quickly to undefined behavior.
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
Also write tests for this function! Since str knows its own size(), does it make sense to throw away that information here? What happens if str contains an embedded '' character — what is string("abc").append(string("de", 3))?
Your use of std::_Is_iterator_v<Iter> is sketchy. I would recommend writing your own portable type-trait, something like this:
template<class, class = void> struct is_iterator : std::false_type {};
template<class T> struct is_iterator<T, decltype(void(std::iterator_traits<T>::iterator_category{}))> : std::true_type {};
Orthogonally, you seem to be using _Is_iterator_v almost like pseudocode that magically does whatever you want done. Here you use it to mean "is an input iterator or better":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
Here you use it to mean "is either kbasic_string::iterator or kbasic_string::const_iterator":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
erase should certainly be rewritten as a non-template:
kbasic_string& erase(const_iterator it)
Finally, consider whether you could save space by replacing every instance of on_heap_ with the condition (capacity_ > 22). And consider moving the magic number 22 into a variable!
static constexpr size_t SBO_CAPACITY = 22;
size_t size_ = 0;
size_t capacity_ = SBO_CAPACITY;
CharType data_[SBO_CAPACITY + 1] = {0};
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
$endgroup$
add a comment |
$begingroup$
It's tricky to review something like this where you're imitating an overcomplicated design, so you know certain things are unimplemented (such as iterators), but then other things might also be unimplemented by accident. For example:
You take a parameter
Traits = std::char_traits<CharType>but then you don't use it! The realbasic_stringwould use the traits class for things like where you usestd::copy.You take an allocator parameter
Allocator = std::allocator<CharType>and do use it, kind of, but you don't go throughstd::allocator_traits<Allocator>. Since all you use isa.allocate()anda.deallocate(), this is probably fine in all real-world situations; but really you should be usingallocator_traits. It can't hurt.You don't store an
Allocatordata member ofkbasic_string; instead, you default-construct one ex nihilo every time you want to use the allocator. This (plus the previous bullet point, technically) means that your container is not allocator-aware: it won't work with user-defined allocators or allocators such asstd::pmr::polymorphic_allocator<char>. You get away with it in your tests right now only becausestd::allocator<char>is (A) empty and (B) default-constructible.You do have tests, right?
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
This is a very strange way of writing
data_ = other.data_;
other.data_ = nullptr;
...oh, I see, your data_ is a char[23], not a char *. Well, okay. I would strongly recommend making it a union of char[23] and char * so that you can copy the pointer member without weird type-punning tricks.
Pedantic nit: Setting all-bits-zero is not technically the same thing as setting to nullptr — the in-memory representation of nullptr need not be all-bits-zero (even though, in practice, it will be).
Also, notice that to be allocator-aware you will have to store a pointer of type std::allocator_traits<Allocator>::pointer, which you definitely cannot assume is all-bits-zero when it's null! For example, boost::interprocess::offset_ptr<char> is not all-bits-zero when it's null.
bool operator==(const kbasic_string& other)
Here and throughout, you forgot the trailing const. These days I recommend making every operator an "ADL friend" non-member:
friend bool operator==(const kbasic_string& other, const kbasic_string& other) {
return std::equal(begin(), end(), other.begin(), other.end());
}
Then it's really obvious if you forget one of the two consts.
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
Write a test case for this function! For example, string("123456789").append(a, a+3). There's a bug in the if condition that leads quickly to undefined behavior.
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
Also write tests for this function! Since str knows its own size(), does it make sense to throw away that information here? What happens if str contains an embedded '' character — what is string("abc").append(string("de", 3))?
Your use of std::_Is_iterator_v<Iter> is sketchy. I would recommend writing your own portable type-trait, something like this:
template<class, class = void> struct is_iterator : std::false_type {};
template<class T> struct is_iterator<T, decltype(void(std::iterator_traits<T>::iterator_category{}))> : std::true_type {};
Orthogonally, you seem to be using _Is_iterator_v almost like pseudocode that magically does whatever you want done. Here you use it to mean "is an input iterator or better":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
Here you use it to mean "is either kbasic_string::iterator or kbasic_string::const_iterator":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
erase should certainly be rewritten as a non-template:
kbasic_string& erase(const_iterator it)
Finally, consider whether you could save space by replacing every instance of on_heap_ with the condition (capacity_ > 22). And consider moving the magic number 22 into a variable!
static constexpr size_t SBO_CAPACITY = 22;
size_t size_ = 0;
size_t capacity_ = SBO_CAPACITY;
CharType data_[SBO_CAPACITY + 1] = {0};
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
$endgroup$
It's tricky to review something like this where you're imitating an overcomplicated design, so you know certain things are unimplemented (such as iterators), but then other things might also be unimplemented by accident. For example:
You take a parameter
Traits = std::char_traits<CharType>but then you don't use it! The realbasic_stringwould use the traits class for things like where you usestd::copy.You take an allocator parameter
Allocator = std::allocator<CharType>and do use it, kind of, but you don't go throughstd::allocator_traits<Allocator>. Since all you use isa.allocate()anda.deallocate(), this is probably fine in all real-world situations; but really you should be usingallocator_traits. It can't hurt.You don't store an
Allocatordata member ofkbasic_string; instead, you default-construct one ex nihilo every time you want to use the allocator. This (plus the previous bullet point, technically) means that your container is not allocator-aware: it won't work with user-defined allocators or allocators such asstd::pmr::polymorphic_allocator<char>. You get away with it in your tests right now only becausestd::allocator<char>is (A) empty and (B) default-constructible.You do have tests, right?
std::memcpy(&data_, &other.data_, sizeof(CharType*));
std::memset(&other.data_, 0, sizeof(CharType*));
This is a very strange way of writing
data_ = other.data_;
other.data_ = nullptr;
...oh, I see, your data_ is a char[23], not a char *. Well, okay. I would strongly recommend making it a union of char[23] and char * so that you can copy the pointer member without weird type-punning tricks.
Pedantic nit: Setting all-bits-zero is not technically the same thing as setting to nullptr — the in-memory representation of nullptr need not be all-bits-zero (even though, in practice, it will be).
Also, notice that to be allocator-aware you will have to store a pointer of type std::allocator_traits<Allocator>::pointer, which you definitely cannot assume is all-bits-zero when it's null! For example, boost::interprocess::offset_ptr<char> is not all-bits-zero when it's null.
bool operator==(const kbasic_string& other)
Here and throughout, you forgot the trailing const. These days I recommend making every operator an "ADL friend" non-member:
friend bool operator==(const kbasic_string& other, const kbasic_string& other) {
return std::equal(begin(), end(), other.begin(), other.end());
}
Then it's really obvious if you forget one of the two consts.
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
{
if (last - first > capacity_)
reserve(last - first);
std::copy(first, last, begin() + size_);
return *this;
}
Write a test case for this function! For example, string("123456789").append(a, a+3). There's a bug in the if condition that leads quickly to undefined behavior.
kbasic_string& append(const kbasic_string& str)
{
return append(str.data());
}
Also write tests for this function! Since str knows its own size(), does it make sense to throw away that information here? What happens if str contains an embedded '' character — what is string("abc").append(string("de", 3))?
Your use of std::_Is_iterator_v<Iter> is sketchy. I would recommend writing your own portable type-trait, something like this:
template<class, class = void> struct is_iterator : std::false_type {};
template<class T> struct is_iterator<T, decltype(void(std::iterator_traits<T>::iterator_category{}))> : std::true_type {};
Orthogonally, you seem to be using _Is_iterator_v almost like pseudocode that magically does whatever you want done. Here you use it to mean "is an input iterator or better":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& append(Iter first, Iter last)
Here you use it to mean "is either kbasic_string::iterator or kbasic_string::const_iterator":
template<typename Iter, typename = std::enable_if_t<std::_Is_iterator_v<Iter>>>
kbasic_string& erase(Iter it)
erase should certainly be rewritten as a non-template:
kbasic_string& erase(const_iterator it)
Finally, consider whether you could save space by replacing every instance of on_heap_ with the condition (capacity_ > 22). And consider moving the magic number 22 into a variable!
static constexpr size_t SBO_CAPACITY = 22;
size_t size_ = 0;
size_t capacity_ = SBO_CAPACITY;
CharType data_[SBO_CAPACITY + 1] = {0};
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
answered Jan 15 at 7:03
QuuxplusoneQuuxplusone
12.3k12061
12.3k12061
add a comment |
add a comment |
$begingroup$
I was looking at a method which I considered to be really easy: c_str, however, it doesn't look that easy based on how you implemented it.
Currently, you have a bool indicating whether you should store it on the heap or not. If you replace that by a pointer to the first character. You could implement the onHeap by comparing 2 pointers. Every access to the characters can simply use the pointer, unless for the appending.
answered Jan 15 at 7:19
JVApenJVApen
690213
$endgroup$
add a comment |
$begingroup$
I was looking at a method which I considered to be really easy: c_str, however, it doesn't look that easy based on how you implemented it.
Currently, you have a bool indicating whether you should store it on the heap or not. If you replace that by a pointer to the first character. You could implement the onHeap by comparing 2 pointers. Every access to the characters can simply use the pointer, unless for the appending.
answered Jan 15 at 7:19
JVApenJVApen
690213
$endgroup$
add a comment |
$begingroup$
I was looking at a method which I considered to be really easy: c_str, however, it doesn't look that easy based on how you implemented it.
Currently, you have a bool indicating whether you should store it on the heap or not. If you replace that by a pointer to the first character. You could implement the onHeap by comparing 2 pointers. Every access to the characters can simply use the pointer, unless for the appending.
answered Jan 15 at 7:19
JVApenJVApen
690213
$endgroup$
I was looking at a method which I considered to be really easy: c_str, however, it doesn't look that easy based on how you implemented it.
Currently, you have a bool indicating whether you should store it on the heap or not. If you replace that by a pointer to the first character. You could implement the onHeap by comparing 2 pointers. Every access to the characters can simply use the pointer, unless for the appending.
answered Jan 15 at 7:19
JVApenJVApen
690213
answered Jan 15 at 7:19
JVApenJVApen
690213
answered Jan 15 at 7:19
JVApenJVApen
690213
answered Jan 15 at 7:19
JVApenJVApen
690213
690213
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f211515%2fc-implementation-of-basic-string%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
a small thing: since is c++ it should be std::size_t not size_t
$endgroup$
– Sandro4912
Jan 15 at 20:45