复制在我的 string::copy 实现中不起作用

Posted 2023-02-22

【中文标题】复制在我的 string::copy 实现中不起作用

【英文标题】：Copy doesn't work in my string::copy implementation

【发布时间】：2016-04-21 18:55:08

【问题描述】：

跟进https://codereview.stackexchange.com/q/126242/23788。

我写了我的字符串类，根据反馈我改变了一些东西。还有什么需要修复的吗？

+operator 不起作用，我不知道我做错了什么。执行“Str+Str”时出现段错误。

Process finished with exit code 139

这是我的 Str.h

class Str 

    friend std::istream &operator>>(std::istream &, Str &);

    friend void swap(Str &s, Str &t) 
        std::swap(s.data, t.data);
        std::swap(s.length, t.length);
        std::swap(s.alloc, t.alloc);
    


public:
    typedef char *iterator;
    typedef size_t size_type;

    Str() : data(nullptr), length(0), capacity(0)  

    Str(size_type length, char char_to_fill) : Str()  create(length, char_to_fill); 

    Str(const char *s) : Str()  create(s); 

    template<class In>
    Str(In b, In e) : Str()  create(b, e); 

    ~Str() 
        if (data) alloc.deallocate(data, capacity);
        data = nullptr;
    

    Str(const Str &s) 
        *this = s;
    

    // move constructor?
    Str(Str &&other)
            : Str() // initialize via default constructor, C++11 only
        swap(*this, other);
    

    Str &operator+=(const Str &s) 
        size_type new_length = length + s.length - 1; //remove 1 because of 2 nulls
        if (new_length > capacity) 
            reallocate(new_length);
            strcpy(data + length - 1, s.data); //overwrite null from s
            length = new_length;
        
        else //if there was already enough space
            strcpy(data + length - 1, s.data);
        
        return *this;
    

    Str &operator=(Str rhs) 
        swap(*this, rhs);
        return *this;
    

    char &operator[](size_type i)  return data[i]; ;

    const char &operator[](size_type i) const  return data[i]; ;

    size_type size()  return length; 

    const size_type size() const  return length; 

    const char *c_str() const 
        return data;
    

    void copy(char *dest, size_type n) 
        if (n > length)
            throw std::out_of_range("Out of range");
        std::copy(data, data + n, dest);
    

    char *begin()  return data; ;

    char *end()  return data + length; ;

    void push_back(char c) 
        if (length == capacity) 
            reallocate(capacity == 0 ? DEFAULT_CAPACITY : 2 * capacity);
        
        data[length++] = c;
    

private:
    char *data;
    std::allocator<char> alloc;
    size_type length;
    size_type capacity;
    static const size_type DEFAULT_CAPACITY = 20;

    void create(size_type n, char character_to_fill) 
        capacity = length = n + 1;
        data = alloc.allocate(capacity);
        std::uninitialized_fill(data, data + length - 1, character_to_fill);
        //alloc.construct(data + length - 1, '\0'); //is it needed to be constructed?
        data[length - 1] = '\0';
    

    void create(const char *s) 
        capacity = length = strlen(s) + 1;
        data = alloc.allocate(capacity);
        strcpy(data, s);
        //alloc.construct(data + length - 1, '\0');
        data[length - 1] = '\0';
    

    template<class In>
    void create(In b, In e) 
        capacity = e - b + 1;
        data = alloc.allocate(capacity);
        while (b != e) 
            data[length++] = *(b++);
        
        //alloc.construct(data + length -1, '\0');
        data[length++] = '\0';
    

    void reallocate(size_t new_capacity) 
        char *new_data = alloc.allocate(new_capacity);
        std::copy(data, data + length, new_data);
        alloc.deallocate(data, length);
        data = new_data;
        capacity = new_capacity;
    
;

std::istream &operator>>(std::istream &is, Str &s) 
    std::vector<char> buf;
    char actual_character;
    while (is.get(actual_character) && isspace(actual_character))  ;
    
    if (is)  //is it correct to check "is" ?
        do buf.push_back(actual_character);
        while (is.get(actual_character) && !isspace(actual_character));
        if (is)
            is.unget();
    
    s.create(buf.begin(), buf.end());
    return is;


std::ostream &operator<<(std::ostream &os, const Str &s) 
    os << s.c_str();
    return os;


Str operator+(Str lhs, const Str &rhs) 
    lhs += rhs;
    return lhs;

还有例子 main.cpp

#include <iostream>
#include <vector>
#include "Str.h"

using std::cout;
using std::endl;

int main() 
    Str s("Siema");
    cout<<s.c_str()<<endl;

    s = "Hello";
    cout<<s<<endl;

    s.push_back('a');
    cout<<s<<endl;

    Str t = "World";
    //cout<<s+t<<endl; //THIS DOESNT WORK

    s+=t;
    cout<<s<<endl;

    cout<<s[3]<<s[5]<<s[11]<<endl;

    cout<<s.size()<<endl;
    cout<<Str(s.begin()+3, s.end()-2)<<endl;
    for(Str::iterator i = s.begin(); i<s.end() ; i+=2)
        cout<<i<<endl;
    

    char copied[3];
    t.copy(copied, 4);
    cout<<copied<<endl;

    return 0;

【问题讨论】：

代码审查和堆栈溢出对于什么是格式良好的问题有非常不同的标准；请阅读minimal reproducible example 和How to Ask。

可能重复：***.com/questions/36364191/…

为什么你的拷贝构造函数调用赋值运算符，它调用拷贝构造函数（参数传值），它调用赋值运算符，它调用拷贝构造函数...？对我来说似乎是一个无限循环。您的复制构造函数应该在没有赋值运算符的帮助下编写。那么在赋值运算符中就可以使用copy/swap了。

【参考方案1】：

在您的代码中

char copied[3];
t.copy(copied, 4);
cout<<copied<<endl;

“已复制”只有 3 个长度，而您尝试将 4 个字符复制到其中。这会导致问题

检查下面的更新代码。用 " 读取 cmets

str.h

#include <iostream>
#include <memory>
#include <vector>
class Str 

friend std::istream &operator >> (std::istream &, Str &);

 void swap(Str &s, Str &t) 
    std::swap(s.data, t.data);
    std::swap(s.length, t.length);
    std::swap(s.alloc, t.alloc);



public:
typedef char *iterator;
typedef size_t size_type;

Str() : data(nullptr), length(0), capacity(0)  

Str(size_type length, char char_to_fill) : Str()  create(length, char_to_fill); 

Str(const char *s) : Str()  create(s); 

template<class In>
Str(In b, In e) : Str()  create(b, e); 

~Str() 
    if (data) alloc.deallocate(data, capacity);
    data = nullptr;


Str(const Str &s) 
    *this = s;


// move constructor?
Str(Str &&other)
    : Str() // initialize via default constructor, C++11 only
    swap(*this, other);


Str &operator+=(const Str &s) 
    size_type new_length = length + s.length - 1; //remove 1 because of 2 nulls
    if (new_length > capacity) 
        reallocate(new_length);
        strcpy(data + length - 1, s.data); //overwrite null from s
        //length = new_length;  //<-- You need to update the length anyay. Move it to before return
    
    else //if there was already enough space
        strcpy(data + length - 1, s.data);
    
    length = new_length; //<-- update the length
    return *this;


Str &operator=(Str rhs) 
    swap(*this, rhs);
    return *this;


char &operator[](size_type i)  return data[i]; ;

const char &operator[](size_type i) const  return data[i]; ;

size_type size()  return length; 

const size_type size() const  return length; 

const char *c_str() const 
    return data;


void copy(char *dest, size_type n) 
    if (n > length)
        throw std::out_of_range("Out of range");
    std::copy(data, data + n, dest);  // <--forgot about '\0'?
    dest[n] = '\0';                   // <-- add '\0'


char *begin()  return data; ;

char *end()  return data + length; ;

void push_back(char c) 
    if (length == capacity) 
        reallocate(capacity == 0 ? DEFAULT_CAPACITY : 2 * capacity);
    
    data[length++ - 1] = c;  //<-- length - 1 is the last position, because length here includes '\0'
    data[length - 1] = 0;    //<-- don't forget to add '\0'. It's better if you fill the unused spaces to '\0' after allocate them.


private:
char *data;
std::allocator<char> alloc;
size_type length;
size_type capacity;
static const size_type DEFAULT_CAPACITY = 20;

void create(size_type n, char character_to_fill) 
    capacity = length = n + 1;
    data = alloc.allocate(capacity);
    std::uninitialized_fill(data, data + length - 1, character_to_fill);
    //alloc.construct(data + length - 1, '\0'); //is it needed to be constructed?
    data[length - 1] = '\0';


void create(const char *s) 
    capacity = length = strlen(s) + 1;
    data = alloc.allocate(capacity);
    strcpy(data, s);
    //alloc.construct(data + length - 1, '\0');
    data[length - 1] = '\0';


template<class In>
void create(In b, In e) 
    capacity = e - b + 1;
    data = alloc.allocate(capacity);
    while (b != e) 
        data[length++] = *(b++);
    
    //alloc.construct(data + length -1, '\0');
    data[length++] = '\0';


void reallocate(size_t new_capacity) 
    char *new_data = alloc.allocate(new_capacity);
    std::copy(data, data + length, new_data);
    alloc.deallocate(data, length);
    data = new_data;
    capacity = new_capacity;

;

std::istream &operator >> (std::istream &is, Str &s) 
std::vector<char> buf;
char actual_character;
while (is.get(actual_character) && isspace(actual_character)) 
    ;

if (is)  //is it correct to check "is" ?
    do buf.push_back(actual_character);
    while (is.get(actual_character) && !isspace(actual_character));
    if (is)
        is.unget();

s.create(buf.begin(), buf.end());
return is;


std::ostream &operator<<(std::ostream &os, const Str &s) 
os << s.c_str();
return os;


Str operator+(Str lhs, const Str &rhs) 
lhs += rhs;
return lhs;

和主要的：

int main() 
Str s("Siema");
cout << s.c_str() << endl;

s = "Hello";
cout << s << endl;

s.push_back('a');
cout << s << endl;

Str t = "World";
//cout<<s+t<<endl; //THIS DOESNT WORK

s += t;
cout << s << endl;

cout << s[3] << s[5] << s[11] << endl;

cout << s.size() << endl;
cout << Str(s.begin() + 3, s.end() - 2) << endl;
for (Str::iterator i = s.begin(); i<s.end(); i += 2) 
    cout << i << endl;


char copied[5];  //<-- was 3, not enough space
t.copy(copied, 4);
cout << copied << endl;

return 0;