0

これが受け入れられる慣行であるかどうか疑問に思っていました：

struct Item { };
std::list<std::shared_ptr<Item>> Items;
std::list<std::shared_ptr<Item>> RemovedItems;

void Update()
{
    Items.push_back(std::make_shared<Item>()); // sample item

    for (auto ItemIterator=Items.begin();ItemIterator!=Items.end();ItemIterator++)
    {
        if (true) { // a complex condition, (true) is for demo purposes
            RemovedItems.push_back(std::move(*ItemIterator)); // move ownership
            *ItemIterator=nullptr; // set current item to nullptr
        }

        // One of the downsides, is that we have to always check if
        // the current iterator value is not a nullptr
        if (*ItemIterator!=nullptr) {
            // A complex loop where Items collection could be modified
        }
    }

    // After the loop is done, we can now safely remove our objects

    RemovedItems.clear(); // calls destructors on objects

    //finally clear the items that are nullptr
    Items.erase( std::remove_if( Items.begin(), Items.end(),
        [](const std::shared_ptr<Item>& ItemToCheck){
            return ItemToCheck==nullptr;
    }), Items.end() );
}

ここでの考え方は、Items コンテナが外部ソースの影響を受ける可能性があることを示しているということです。アイテムがコンテナーから削除されると、単純に nullptr に設定されますが、その前に RemovedItems に移動されます。

イベントのようなものが影響し、Itemsアイテムを追加/削除する可能性があるため、この解決策を考え出す必要がありました。

これは良い考えだと思いますか？

4

3 に答える 3

2

あなたは物事を複雑にしすぎていると思います。マルチスレッドの状況にある場合 (質問で言及していません)、変更されたリストにアクセスする他のスレッドからの読み取りを保護するロックが必要になることは確かです。標準ライブラリには並行データ構造がないため、そのようなものを自分で追加する必要があります。

シングルスレッド コードの場合は、述語を使用してstd:listメンバーを呼び出すだけです。remove_ifポインターを null に設定して保存し、データに対して複数のパスを実行する必要はありません。

#include <algorithm>
#include <list>
#include <memory>
#include <iostream>

using Item = int;

int main()
{
    auto lst = std::list< std::shared_ptr<Item> > 
    { 
        std::make_shared<int>(0), 
        std::make_shared<int>(1), 
        std::make_shared<int>(2), 
        std::make_shared<int>(3),         
    };    

    // shared_ptrs to even elements
    auto x0 = *std::next(begin(lst), 0);
    auto x2 = *std::next(begin(lst), 2);

    // erase even numbers
    lst.remove_if([](std::shared_ptr<int> p){
        return *p % 2 == 0;    
    });

    // even numbers have been erased
    for (auto it = begin(lst); it != end(lst); ++it)
        std::cout << **it << ",";    
    std::cout << "\n";

    // shared pointers to even members are still valid
    std::cout << *x0 << "," << *x2;
}

ライブの例。

要素がリストの最後に置かれただけでなく、リストから実際に削除されたことに注意してください。後者の効果は、標準アルゴリズムが行うことであり、その後でメンバー関数std::remove_ifを呼び出す必要があります。この2段階の消去-削除イディオムは次のようになりますstd::listerase

// move even numbers to the end of the list in an unspecified state
auto res = std::remove_if(begin(lst), end(lst), [](std::shared_ptr<int> p){
    return *p % 2 == 0;    
});

// erase even numbers
lst.erase(res, end(lst));

ライブの例。

ただし、どちらの場合も、基になるItem要素は削除されていません。これは、それぞれに関連付けられた共有ポインターがまだあるためです。参照カウントがゼロになった場合にのみ、それらの以前のリスト要素が実際に削除されます。

于 2013-10-07T12:30:07.017 に答える
1

もし私がこのコードを見直していたら、それは受け入れられないと言うでしょう。

二段階除去の目的は何ですか？そのような異常な決定には、その目的を説明するコメントが必要です。繰り返し要求されたにもかかわらず、あなたはその要点を説明できませんでした。

ここでの考え方は、Itemsコンテナが外部ソースによって影響を受ける可能性があることを示しているということです。

「ここでの考え方は、Items コンテナーをマークしている間、外部ソースによって影響を受ける可能性があるということです。」? そうでなければ、その文は意味をなしません。

それはどのように影響を受ける可能性がありますか？あなたの説明は明確ではありません：

関係を考えてくださいRoot -> Parent -> Child。からChild削除できるでイベントがトリガーされる場合があります。そのため、ループが途中で壊れ、反復子が無効になる可能性があります。ParentRoot

それは何の説明にもなりません。非常に広い用語を使用しているため、あまりにも漠然としています。あなたが何を意味するのか説明してください。

「親子関係」には、さまざまな意味があります。タイプが継承によって関連しているということですか？オブジェクトは所有権によって関連していますか? 何？

どんな「イベント」？イベントは多くのことを意味する可能性があります.StackOverflowの人々が「イベント」という言葉を特定の意味で使用するのをやめ、他の誰もが意図する意味を知っていると仮定してほしい. 別のスレッドなどでの非同期イベントのことですか? それとも、 を破棄すると、リストItemから他の要素が削除される可能性があるということですか?Items

非同期イベントを意味する場合、ソリューションは問題に完全に対処できません。コンテナを同時に変更できる場合、標準コンテナを安全に反復処理することはできません。これを安全に行うには、変更中にコンテナーへの排他的アクセスを確保するために何かを行う必要があります (例: ミューテックスをロックする)。

このコメントに基づいて：

// A complex loop where Items collection could be modified

非同期イベントを意味するのではないと思います（しかし、なぜ「外部ソース」がコンテナを変更できると言うのですか）。Itemイテレータを有効に保つだけでなく、実際のオブジェクトを有効に保つ必要がありますか? 要素をnullptr に入れずに に設定して、最後RemovedItemsに行うことはできませんでしItems.remove_if([](shared_ptr<Item> const& p) { return !p; }たか? 「複雑なループ」がコンテナーまたはアイテムに対して実行できることについて、もう少し説明する必要があります。

関数RemovedItemsにローカル変数がないのはなぜですか? Update()その関数の外では必要ないようです。新しい C++11 範囲ベースのforループを使用して、リストを反復処理してみませんか?

最後に、なぜすべて大文字で名前が付けられているのですか?! ローカル変数や関数に大文字で名前を付けるのは奇妙です。すべてがそのように名前付けされている場合、大文字はさまざまな種類の名前を区別するのに役立たないため、意味がありません (たとえば、種類だけに大文字を使用すると、どの名前が適切であるかが明確になります)。タイプであり、そうではない...すべてに使用しても意味がありません。)

于 2013-10-07T14:16:36.160 に答える