2016-04-19 20 views
-1

std :: list oneのようなリストのテンプレートを書こうとしています。 これはList.hで私のコードです:私のstd :: list実装のイテレータは動作しません

#include <memory> 
#include <cassert> 
#include <iterator> 

template<typename T, class Node> 
class iterator : public std::iterator<std::bidirectional_iterator_tag, Node *, Node &> { 
    Node *underlying; 

public: 
    explicit iterator(Node *n) : underlying(n) { }; 

    iterator() : underlying(nullptr) { }; 

    iterator &operator++() { //preinc 
    assert(underlying != nullptr && "Out-of-bounds iterator increment!"); 
    underlying = underlying->next; 
    return *this; 
    } 

    iterator operator++(int) { //postinc 
    assert(underlying != nullptr && "Out-of-bounds iterator increment!"); 
    iterator temp(*this); 
    ++(*this); 
    return temp; 
    } 

    iterator &operator--() { //predec 
    assert(underlying != nullptr && "Out-of-bounds iterator decrement!"); 
    underlying = underlying->previous; 
    return *this; 
    } 

    iterator operator--(int) { //postdec 
    assert(underlying != nullptr && "Out-of-bounds iterator decrement!"); 
    iterator temp(*this); 
    --(*this); 
    return temp; 
    } 

    bool operator==(const iterator &rhs) { 
    return underlying == rhs.underlying; 

    } 

    bool operator!=(const iterator &rhs) { 
    return underlying != rhs.underlying; 

    } 

    T &operator*() { 
    return underlying->data; 
    } 
}; 

template<typename T> 
class List { 

    class Node { 
    public: 
    T data; 
    Node *previous; 
    Node *next; //is that T needed? 
    Node(T &d) : data(d) { }; 
    }; 

private: 

    Node *head; //first element 
    Node *tail; 

    void create() { head = tail = NULL; } 

    void create(const List &rhs) { 
    iterator this_iter = head; 
    iterator rhs_iter = rhs.head; 
    while (rhs_iter != NULL) { 
     this_iter->data = (rhs_iter++)->data; 
     ++this_iter; 
    } 
    }; 

public: 
    typedef T *iterator; 
    typedef const T *const_iterator; 
    typedef size_t size_type; 
    typedef T value_type; 

    List() { create(); }; 

    List &operator=(const List &rhs) { 
    if (&rhs != this) { 
     create(rhs); 
    } 
    return *this; 
    }; 

    List(const List &rhs) { create(rhs); }; 

    ~List() { while(head) remove(head); }; 

    T *begin() { return head; }; 

    T *end() { return tail; }; 

    T front() { return head->data; }; 

    T back() { return tail->data; }; 

    bool empty() { return head == NULL; } 

    size_type size() { 
    size_t i = 0; 
    Node *node = head; 
    while (node) { 
     node = node->next; 
     i++; 
    } 
    return i; 
    }; 

    T &operator[](size_type i) { 
    if (i < size() && i >= 0) { 
     Node *temp = head; 
     while (i > 0) { 
      temp = temp->next; 
      i--; 
     } 
     return temp->data; 
    } 
    throw std::out_of_range("Index out of range"); 
    }; 

// const T &operator[](size_type i) const; //how to implement and do not duplicate code? 

    Node *push_back(value_type data) { 
    Node *n = new Node(data); 
    if (head == NULL) { 
     head = tail = n; 
    } else { 
     n->previous = tail; 
     tail->next = n; 
     tail = n; 
    } 
    return n; 
    }; 

    Node *push_front(value_type data) { 
    Node *n = new Node(data); 
    if (head == NULL) { 
     head = tail = n; 
    } else { 
     n->next = head; 
     head->previous = n; 
     head = n; 
    } 
    return n; 
    }; 

    void pop_front() { 
    remove(head); 
    }; 

    void pop_back() { 
    remove(tail); 
    }; 

    void remove(Node *n){ 
    if(n == NULL) return; 
    if(n == head){ 
     head = n->next; 
     head->previous =NULL; 
    } 
    else if(n == tail){ 
     tail = n->previous; 
     tail->next = NULL; 
    } 
    else{ 
     n->previous->next = n->next; 
     n->next->previous = n->previous; 
    } 
    delete n; 
    } 


}; 

そして、これはmain.cppに

#include <iostream> 
#include "List.h" 
int main(){ 
    List<int> l; 
    l.push_back(1); 
    l.push_back(2); 
    l.push_back(3); 
    l.pop_back(); 
    l.pop_front(); 
    l.push_back(4); 
    l.push_back(5); 
    for (size_t i = 0; i < l.size(); i++) 
     std::cout << l[i] << "\n"; 
    std::cout<<"Front "<<l.front(); 
    std::cout<<"Back "<<l.back(); 
} 

実は一back /フロント、pop_back /フロントと[]オペレータの作業罰金です。しかし、フロント()またはバック()を使用しようとすると「終了コード139で処理が完了しました」というメッセージが表示されます。 エラーそして、私はリストテンプレートのこのイテレータが動作しないことを知っていますが、私はそれを組み合わせる方法を知っています。誰かがヒントや助けてもらえますか?

EDIT: Ok、私はremoveとfront()、tail()メソッドの問題を修正しました。しかし、イテレータのことはまだ動作しません。 たとえば、このコード:

for(List<int>::iterator it = l.begin(); it!=l.end(); it++){ 
     std::cout << it << "\n"; 
    } 

は私にerrosを与える:

error: cannot convert ‘List<int>::Node*’ to ‘List<int>::iterator {aka int*}’ in initialization 
    for(List<int>::iterator it = l.begin(); it!=l.end(); it++){ 
             ^
error: comparison between distinct pointer types ‘List<int>::iterator {aka int*}’ and ‘List<int>::Node*’ lacks a cast [-fpermissive] 
    for(List<int>::iterator it = l.begin(); it!=l.end(); it++){ 
                ^

私はテ問題はイテレータテンプレートでノードをwrapingであると私は「型名Tの*イテレータ」を持っていることを知っています。

+0

あなたの 'Node(T&d)'は 'next'も' previous'も設定しません...おそらく大きな問題はありません – PiotrNycz

+0

このコードをテストしてください!空リストを作成する - 空であるかどうかをチェックする - 関数を呼び出して、すべてが正しく動作するかどうかを確認する。 1つの要素でリストを作成する - リスト関数を呼び出す - すべてが機能することを確認する。 2つの要素Listに対して同じことを行います。その他... – PiotrNycz

+0

あなたの問題をまだ分かっていないのであれば、誰かが助けることができるようになる前に、それを再現する最小限のプログラムを構築する必要があります。 – Useless

答えて

1

問題(問題?)remove()である:tailNULLある場合headは、(headケース)NULLある場合はチェックしません(tailケース)とn->previousn->nextはヌル(一般的なケース)であれば

私はこのremove()

void remove(Node *n){ 
    if(n == NULL) return; 
    if(n == head){ 
     head = n->next; 
     if (head) 
     head->previous =NULL; 
    } 
    else if(n == tail){ 
     tail = n->previous; 
     if (tail) 
     tail->next = NULL; 
    } 
    else{ 
     if (n->previous) 
     n->previous->next = n->next; 
     if (n->next) 
     n->next->previous = n->previous; 
    } 
    delete n; 
} 
3

あなたbeginend方法はNode*ではなく、あなたのITERを返す示唆アタータイプ。引数としてNode*を受け入れるイテレータコンストラクタを作成しました。explicit; は、List<int>::Node*からList<int>::iteratorへの暗黙的な変換が許可されていないコンパイラに対してと言いました。

次のいずれかの作業を行う必要がありますiterator(head)iterator(tail)(明示的な変換を実行を返すように変更beginend

  • (これはシナリオで暗黙の型変換をリスクかかわらず、あなたがしたくない) explicit iterator(Node *n) : underlying(n) { };から explicitを削除

    1. headtailではなく、beginendの戻り値の型をiteratorに変更してください。これは本当に関係なく行う必要があります)

    あなたも他のいくつかの問題があります:あなたはListtypedef T* iteratorを行っているべきではありません

    1. 。あなたのiteratorクラスの定義を隠していたので、Listは決して使用しませんでした。それを修正して(iteratorの用途に必要なテンプレートを追加すると)、コンパイルされます
    2. iteratorの継承の定義はtemplate<typename T, class Node> class iterator : public std::iterator<std::bidirectional_iterator_tag, T>ではなく、template<typename T, class Node> class iterator : public std::iterator<std::bidirectional_iterator_tag, Node *, Node &>です。後者はiterator<T, Node>(nullptr)ないiterator(tail)を返す必要があります
    3. end(あなたはそれがT、各Nodeの値にしたいとき)Node *する必要がありますデリファレンスから値を宣言しています。そうでなければ、tailの値を印刷する前にループを終了する前にtailを印刷します。

    すべての作業が完了したら、コンパイルして結果を得る必要があります。コードにはまだ問題があります。

    1. それは正しくないconstだと最適化を防ぐことができ、さまざまなアクセサのないconstバージョンを提供しています。コンパイラはループがあることを把握することはできません場合は、すべてのループ上sizeを再計算に終わる可能性があり実際非変異
    2. createが動作しないコピーコンストラクタ/代入ユーティリティ関数は、(あなたは、反復処理したいと思いますrhspush_back繰り返し、あなたがconstアクセサの欠如はユーティリティー機能が動作させることができないことを意味し、新しい値
    3. にプッシュするiteratorを使用することはできません。それは保証してrhsを反復するconstイテレータ型が必要になりますconst List&の要件に違反することはありませんが、反復子関数の変更は定義されています。

    しかし、あなたが運動していないコードのパフォーマンスの最適化と正確さ。新しいコードに満足したら修正することができます。

    私はyour codeを持っています(ただし、建設/割り当てをコピーするための修正は、const安全反復の欠如の周りに力を入れてconst_castを使用する際立ったハックです)。また、コピーの作成と割り当ての作業を示すためにいくつかのテストを追加しました。 Take a look

    +0

    @Mateusz: 'begin'と' end'の戻り値の型を一致させるのを忘れました。彼らは 'Node *'ではなく 'iterator'を返す必要があります。コンパイルして実行する修正。 – ShadowRanger

    +0

    ええと、まだ同じエラー。 http://cpp.sh/2ln7あなたが* begin()と* end()メソッドを使用したと仮定します。 – Mateusz

    +0

    @Mateusz: '*'を残しました。 'iterator *'を返さず、 'iterator '。 'Node'のコピーを返すのは無意味ですが、' iterator'( 'Node *'の周りの薄いラッパー)では、参照/ポインタではなく値で返りたいので、 'Node *'を返すのです。つまり、 'Lister'の定義のために' iterator'に取って代わった 'typedef'を使用していたことに気づいたので、あなたは' iterator'クラスを使用していませんでした。他にも多くの問題がありました。私はそれらの多くに対処する答えを更新しました。 – ShadowRanger

    関連する問題