0

二つの連結リストを結合させ(一つの連結リストをもう片方の連結リストにつなげる)、その結合したリストを表示させるプログラムを作りました。

append(TextClass otherList)ではダメでappend(TextClass const& otherList)じゃないといけないみたいなのですがこれの理由がはっきりとわかっていません。なぜ、append(TextClass otherList)ではうまくいかないのでしょうか。

main.cpp

#include <iostream>
#include "TextClass.h"

using namespace std;

int main() {
    const int APPEND1 = 6;
    const int APPEND2 = 7;
    int counter = 0;
    char appendVals[APPEND1 + APPEND2] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm'};

    TextClass first;
    for (int i = 0; i < APPEND1; i++)
    {
        first.addTail(appendVals[counter]);
        counter++;
    }
    TextClass second;
    for (int i = 0; i < APPEND2; i++)
    {
        second.addTail(appendVals[counter]);
        counter++;
    }

    std::cout << "First list is " << first.displayList() << std::endl;
    std::cout << "Second list is " << second.displayList() << std::endl;

    first.append(second);
    std::cout << "First should now be a b c d e f g h i j k l m" << std::endl;
    std::cout << " and it actually is " << first.displayList() << std::endl;

    std::cout << "Done appending a list\n" << std::endl;


    return 0;
}

TextClass.cpp

#include "TextClass.h"
#include <sstream>
//constructor
Link::Link(char letter, Link* next, Link* prev)
{
    value = letter;
    this->next = next;
    this->prev = prev;
}

//destructor
Link::~Link(){}

//return the value in the link
char Link::getValue(){return value;}

//set a new next address
void Link::setNext(Link* next){this->next = next;}

//set a new previous address
void Link::setPrev(Link* prev){this->prev = prev;}

// return the next address
Link* Link::getNext(){return this->next;}

// return the previous address
Link* Link::getPrev(){return this->prev;}


//constructor
TextClass::TextClass()
{
    //head and tail are set to nullptr
    head = nullptr;
    tail = nullptr;
}

//destructor
TextClass::~TextClass()
{
    //call removeHead until head = nullptr;
    while(head != nullptr)
    {
        removeHead();
    }
}


//add value at tail
void TextClass::addTail(char letter)
{
    //if head == nullptr
    if(tail == nullptr)
    {
        //head and tail = new link
        head = tail = new Link(letter);
    }
        //else
    else
    {
        //create new link and insert it at tail
        Link* temp = new Link(letter, nullptr, tail);
        tail->setNext(temp);
        //change tail
        tail = temp;
    }
}


//return the contents of list
string TextClass::displayList()
{

    //create a variable that stores contents for string and stringstream
    string output;
    std::stringstream ss;
    //create a link that walks down the list and it starts from head
    Link* temp = head;
    //while the link != nullptr
    while(temp != nullptr)
    {
        //copy the value in the link to stringstream
        ss << temp->getValue() << " ";
        //move the link forward
        temp = temp->getNext();
    }

    //copy the value in stringstream to string
    output = ss.str();

    //return the string
    return output;
}

//connects two lists
void TextClass::append(TextClass const& otherList)
{
    //create a link that stores the head of otherList
    Link* temp = otherList.head;

    //connect two links
    //tail next should point to head of otherLink
    while(temp != nullptr)
    {
        addTail(temp->getValue());
        temp = temp->getNext();
    }

}

//remove head
void TextClass::removeHead()
{
    //save the link at head to delete later
    Link* temp = head;
    //update head
    head = head->getNext();
    //if head is null after update
    if(head == nullptr)
    {
        //tail = nullptr
        tail = nullptr;
    }
        //else update prev of new link
    else
    {
        head->setPrev(nullptr);
    }
    //now delete the old link
    delete temp;
}

TextClass.h

#include <iostream>


using std::string;

class Link {
private:
    char value; //stores chat type value
    Link* next; //stores the address of next link
    Link* prev; //stores the address of previous link

public:
    Link(char letter, Link* next = nullptr, Link* prev = nullptr); //constructor
    ~Link(); //destructor
    char getValue(); //return the value in the link
    void setNext(Link* next); //set a new next address
    void setPrev(Link* prev); //set a new previous address
    Link* getNext(); // return the next address
    Link* getPrev(); // return the previous address
};


class TextClass {
private:
    Link* head; //track the head link of queue
    Link* tail; //track the tail link of queue
public:
    TextClass(); //constructor
    ~TextClass(); //destructor

    void addTail(char letter); //add value at tail
    string displayList(); // return the contents of list
    void append(TextClass const& otherList); //connects two lists
    void removeHead(); //remove head
};

2 件の回答 2

1

提示 TextClass はデストラクタで自分の持つリスト要素を全部 delete しています。別の言い方すると TextClass インスタンス自体とその中のリスト要素は寿命を共にします。つまり他の TextClass インスタンスとリスト要素を共用してしまうと誤動作します。

議題のコピーコンストラクタですが JIS X 3014:2003 (古くてごめん) の 12.8 クラスオブジェクトのコピー によると

12.8-4 クラスの定義がコピーコンストラクタを明示的に宣言しない場合、コピーコンストラクタが暗黙に宣言される。
12.8-8 暗黙に定義されたコピーコンストラクタは、その部分オブジェクトをメンバーごとにコピーする

とあります。提示例 TextClass のメンバは単純にポインタなので、ポインタ値のコピーが行われます。俗にいうところのシャローコピーを行うわけです。結果的に複写元と複写先でポインタ値は同じになり、先の前提「異インスタンスでリスト要素を共有してはならない」に反してしまいます。

append(TextClass) だと、この引数は値渡し、つまりここで(暗黙の)コピーコンストラクタが起動されて複写が行われます。つまり main 内部の secondappend 内での仮引数 otherList は暗黙のコピーコンストラクタによってコピーされた別インスタンスなのでリスト要素を共有してしまっているわけです。

複写されたほうの otherList は、その関数呼び出しを含む完結式の完了時点で破棄されます C++で関数の引数リスト内で一時オブジェクトを生成した時のデストラクタが呼び出されるタイミング ので、この際にリスト要素は delete されてしまいます。 second から見ると自分が持っているポインタの指す先がなくなっているという典型的ダングリングポインタ事例となってしまっています。

参照渡し append(TextClass&) だと複写がないつまり破棄もないので問題になりません。

他の解決方法はとしては例えば
- まじめにディープコピーを行う TextClass::TextClass(const TextClass&)
- メンバをスマートポインタにする std::shared_ptr
あたりが挙げられそうですが、前者は今回の目的には過剰すぎる=無駄に重い処理をして遅いだけだし、後者は学習中の今の時点ではまだ早すぎるかもしれません。

2
  • とても、わかりやすい説明ありがとうございました。リストを共有してはいけないこと、誤のotherListは関数の終わりでデストラクトされてしまうことは知りませんでした。大変参考になりました。 Commented 2021年10月18日 4:44
  • 一般論として「リストを共有してはいけない」わけではなくて 今の実装だと 共有してはいけないってことですのでその辺は誤解なきよう。共用できるスマートポインタを使うよう修正する方法はあるよね、と既に書いていますしね。
    – 774RR
    Commented 2021年10月18日 5:57
-1

このページの解説が分かりやすいと思います。

【C++】メンバにポインタを持つクラスの注意点(2重解放、コピーコンストラクタ、代入演算子)

2
  • 二重解放というものが原因でしたか。思わず、なるほどとつぶやいてしまいました。 Commented 2021年10月18日 4:57
  • リンクだけの紹介は回答としては評価が低くなりがちです。リンクは参考サイトとしてこのQAに残すつもりでご自身で回答してみるとよいでしょう。
    – keitaro_so
    Commented 2021年10月25日 0:21

この質問に回答するには、ログインする必要があります。

求めていた回答ではありませんか? のタグが付いた他の質問を参照する。