Qt + вумные указватели

[ViTech]

Ну как-то не очень удобно (особенно по сравнению с простецким "delete item").

Как Вы относитесь к идее не использовать в современном С++ new и delete? Т.е. для создания объекта использовать функции типа make_unique/shared, а удаляться объект будет, когда его умные указатели помрут.

[Авварон]

И, как я понял, это придется делать так

Код

C++ (Qt)
    ItemUniquePointer take(ItemObserverPointer item)
    {
        ItemUniquePointer result;
        const auto pred = [item](const ItemUniquePointer &p) { return item.get() == p.get(); };
        const auto it = std::find_if(m_children.begin(), m_children.end(), pred);
        if (it != m_children.end()) {
            std::swap(result, *it);
            result->m_parent = nullptr;
            m_children.erase(it);
        }
        return result;
    }
 
void DeleteItem( ItemObserverPointer & item )
{
    (void)item->parent()->take(item);
}
 

На самом деле, там есть метод children который дает залезть в кишки напрямую и просто достать ребенка из вектора но он не очень безопасен так как не зануляет парента и не соблюдает инвариант.

Ну как-то не очень удобно (особенно по сравнению с простецким "delete item"). И кстати не видно где/как пресекается удаление непотопляемого root'a

Ну а как вы рута собираетесь удалять? Метода выцепить рут у вьюхи нет, он на то и invisible root item что он всегда есть.
Метод clear() какой-нибудь да, можно добавить, который свапнет текущий рут с пустым и вуаля рут снова всегда есть.

[Авварон]

Я немного обновил код, в частности добавил методы take() и переименовал тайпдефы в более простые варианты (Pointer и Holder)

[ssoft]

Я немного обновил код, в частности добавил методы take() и переименовал тайпдефы в более простые варианты (Pointer и Holder)

Смущает только использование не быстрого поиска дочернего элемента

Код

C++ (Qt)
const auto pred = [item](const ItemUniquePointer &p) { return item.get() == p.get(); };
const auto it = std::find_if(m_children.begin(), m_children.end(), pred);
 

Можно немного изменить ItemObserverPointer - реализовать его через итератор листа, с помощью которого и удалять дочерний элемент без всякого поиска.

[Авварон]

Смущает только использование не быстрого поиска дочернего элемента

Код

C++ (Qt)
const auto pred = [item](const ItemUniquePointer &p) { return item.get() == p.get(); };
const auto it = std::find_if(m_children.begin(), m_children.end(), pred);
 

Можно немного изменить ItemObserverPointer - реализовать его через итератор листа, с помощью которого и удалять дочерний элемент без всякого поиска.

Идея интересная, но проблема в том, что итераторы вектора не стабильны - при добавлении айтема в середину итераторы поедут.
Так-то можно просто в сам айтем положить его "позицию" (итератор или индекс) в паренте но см выше.

[Igors]

Ну а как вы рута собираетесь удалять? Метода выцепить рут у вьюхи нет, он на то и invisible root item что он всегда есть.

До root'a можно добраться через parent().

Код

C++ (Qt)
void DeleteItem( ItemObserverPointer & item )
{
    (void)item->parent()->take(item);
}
 

А если parent'а нет? И занулять item наверно все-таки надо. Так это просто еще один вариант remove (симпатичнее)

Как Вы относитесь к идее не использовать в современном С++ new и delete? Т.е. для создания объекта использовать функции типа make_unique/shared, а удаляться объект будет, когда его умные указатели помрут.

Если уж так хочется принципиально не использовать указатели и адресную арифметику, то для этого есть др языки, напр жаба или пытон. Но сама по себе идея интересна, в конце-концов никто не заставляет использовать такой подход всегда и везде.

Ну хорошо, вот я использую голый и, естественно, получаю за это, напр

Код

C++ (Qt)
qDeleteAll(treeWidget->selectedItems());

Увы, так резво нельзя, это рухнет если и parent и child выбраны. Приходится сначала отфильтровать selection, не смертельно,  но заботы/хлопоты. Ожидается что в новой реализации этого не произойдет, ведь все голые истреблены! Но.. я не вижу как, по-моему проблемы все те же, и фильтровать все равно надо. Ну получим мы вектор ItemObserverPointer, но он (падла), не отслеживает валидность и по-прежнему указывает на айтем что парент давно грохнул.

Тогда где "выйгрышь"? Или я ошибаюсь?

[ViTech]

Если уж так хочется принципиально не использовать указатели и адресную арифметику, то для этого есть др языки, напр жаба или пытон.

Зачем так радикально, если на С++ можно писать так же и даже лучше .

...
Тогда где "выйгрышь"? Или я ошибаюсь?

Про дерево - отдельный разговор. Главное в умных указателях - это понятное управление временем жизни объекта. И бонусом - автоматическое "зануление" weak-указателей. Но вряд ли кто будет пихать в них автоматическое удаление из всевозможных сторонних контейнеров. Это другая задача, с другими реализациями и накладными расходами.

А "выйгрышь" можно увидеть на примере того же Car. В Вашей реализации есть утечки памяти: в main три раза выполнено new, и ни разу delete. Можете сказать, что в данном примере это несущественно, он про другое. Но, тем не менее, ситуация показательная: в Вашем варианте приходится задумываться, что и когда нужно/можно удалять, а что нельзя. Нужно удалять engine и car, и нельзя удалять engine2. Только из кода это ни фига не очевидно . Для сравнения, попробуйте в моём варианте что-нибудь сломать, случайно или намеренно (естественно кроме хаков типа delete &*ptr и пр.). Можно, конечно, набираться профессионализма в отладчике, но лучше просто писать программы, у которых вероятность сломаться намного меньше .

[Igors]

Зачем так радикально, если на С++ можно писать так же и даже лучше .

Конечно, "дай бог", но пока не очень 

Про дерево - отдельный разговор. Главное в умных указателях - это понятное управление временем жизни объекта. И бонусом - автоматическое "зануление" weak-указателей.

Не согласен с такими акцентами. Пресловутый "observer" никак не влияет на время жизни, но иметь с него валидность очень хотелось бы (см только что приведенный пример) 

Но вряд ли кто будет пихать в них автоматическое удаление из всевозможных сторонних контейнеров. Это другая задача, с другими реализациями и накладными расходами.

Другая. Но встречается часто и заметьте, обычно вкупе или рядом с вумными. И выясняется что сам по себе вумный не очень-то умен (валидность есть - и слава богу) - все равно приходится возиться с контейнерами связей (как и в данном случае)

Для сравнения, попробуйте в моём варианте что-нибудь сломать, случайно или намеренно (естественно кроме хаков типа delete &*ptr и пр.).

Какой хороший я - и песенка моя!

Можно, конечно, набираться профессионализма в отладчике, но лучше просто писать программы, у которых вероятность сломаться намного меньше .

Насколько хорош (надежен и.т.п.) мой код - это конечно имеет значение. Но далеко не первостепенное, эти проблемы не идут ни в какое сравнение с тем как вообще получить/написать работающий, ф-циональный код. Др словами обычно на старте я вообще НЕ ЗНАЮ что писать См раздел "Алгоритмы", хотя бы последняя (школьная) задачка. После N дней гугления, возможно, что-то и найдется, но можно ли этому верить и стоит ли связываться с реализацией - хз. Это потом уже, когда по сути все решено, можно и покалякать, а что там лучше "юник" или "шаред". В общем "писать программы" и "синтаксический сахар" - вещи очень разные 

[ViTech]

Пресловутый "observer" никак не влияет на время жизни, но иметь с него валидность очень хотелось бы (см только что приведенный пример)

...

Код

C++ (Qt)
qDeleteAll(treeWidget->selectedItems());

Увы, так резво нельзя, это рухнет если и parent и child выбраны. Приходится сначала отфильтровать selection, не смертельно,  но заботы/хлопоты. Ожидается что в новой реализации этого не произойдет, ведь все голые истреблены! Но.. я не вижу как, по-моему проблемы все те же, и фильтровать все равно надо. Ну получим мы вектор ItemObserverPointer, но он (падла), не отслеживает валидность и по-прежнему указывает на айтем что парент давно грохнул.

Через "observer" нельзя напрямую удалять объект (использовать delete observer и вариации). Как Вам такая мысль?

В общем "писать программы" и "синтаксический сахар" - вещи очень разные 

Код

C++ (Qt)
Engine * Car::ReplaceEngine( Engine * engine )
{
    QMutexLocker lock(&mMutex);
    Q_ASSERT(engine && engine->GetOwner() == 0);
    Engine * old = mEngine.take();
    old->SetOwner(0);
 
    mEngine.reset(engine);
    engine->SetOwner(this);
 
    return old;
}
...
void Monitor::SetEngine( Engine * engine )
{
    QMutexLocker lock(&Car::mMutex);
    if (engine == mEngine) return;
 
    if (mEngine)
        mEngine->RemMonitor(this);
 
    mEngine = engine;
 
    if (mEngine)
        mEngine->AddMonitor(this);
}

vs

Код

C++ (Qt)
SingleEngine Car::replaceEngine(SingleEngine engine)
{
    using std::swap;
    swap(m_engine, engine);
    return engine;
}
...
void Monitor::setEngine(const WeakEngine& engine)
{ m_engine = engine; }

Как по мне, так очень сладкий сахар получается. Вам так не кажется? Впрочем, о вкусах, естественно, не спорят .

[Igors]

Через "observer" нельзя напрямую удалять объект (использовать delete observer и вариации). Как Вам такая мысль?

Кстати это "дыра" в идеологии "без new/delete" - а что делать если нужно именно "удалить"? Напр айтем из списка, таких случаев полно. Ну хорошо, напишем DeleteItem, обратимся к parent, не переломимся. Так ведь все равно рухнет

Как по мне, так очень сладкий сахар получается. Вам так не кажется? Впрочем, о вкусах, естественно, не спорят .

Да сладкий-то он может и сладкий (не мне судить, всегда пью чай без сахара). Только вот нужного ф-ционала он не имеет. Проблема/вопрос "а какие мониторы наблюдают за данной Engine?" возникнет очень быстро, и городить их контейнер (член Engine) придется. Как и отслеживать связки. А оставшиеся вумные указатели будут немым укором несостоявшейся вумности.

[Авварон]

Код

C++ (Qt)
    using std::swap;
 

Кстати начиная с какой-то версии плюсов это не нужно, можно просто std::swap звать, потому что теперь можно пегегружать std::swap и юзерский swap не нужен.

[Авварон]

Кстати это "дыра" в идеологии "без new/delete" - а что делать если нужно именно "удалить"? Напр айтем из списка, таких случаев полно. Ну хорошо, напишем DeleteItem, обратимся к parent, не переломимся. Так ведь все равно рухнет

Я конечно понимаю желание стрелять в ногу, но почему оно должно рухнуть? У вас есть инвариант - либо айтем в дереве и у него есть парент (и его удалить нельзя), либо айтем не в дереве и с ним можно делать что угодно. Единственный способ перехода между инвариантами - добавление\убирание из дерева.
Всё. Других методов просто нет. "delete item" это не убирание из дерева, это удаление. И не надо нагружать удаление этой семантикой - это очень плохой подход.
Вы, наверное, никогда не писали код с исключениями (да откуда они в Qt программе, верно), но код вида new MyObject(this) скорее всего утечет, если ВНЕЗАПНО вылетит bad_alloc при ресайзе вектора детей. Хорошо, если где-то внутри у этот указатель завернется во временный QScopedPointer (что скорее всего не так), но вам не кажется странным заворачивать указатель в юник внутри конструктора если можно просто этот же самый юник создать на стороне юзера?

[Авварон]

У нас кстати вот есть точно такой кусок кода - объекты в конструкторе себя регистрируют в глобальном кэше, при удалении дерегистрируют.
И что вы думаете? Где что удаляется хрен пойми. И да, это говно течет. Ровно потому что где что удаляется хрен пойми.

Ну то есть - вот он есть (глобальный, прости хоспаде) кэш. Он владеет объектами? Вроде бы нет, он не подчищает что осталось в деструкторе и делетов в нем нет. Но при том если объект есть в кэше, то новый создавать не надо, надо взять из кэша. А кто владеет объектами? Да хрен пойми, они тут и там запрашиваются и никто не думает ничего удалять.

И вот я типа могу заюзать shared_ptr и хранить в кэше шареды - всё чики-пуки... Кроме тех мест где таки явно зовется delete object с целью удалить их из кэша

[Igors]

но почему оно должно рухнуть?

Пусть выбраны 2 айтема и первый - парент второго

Код

C++ (Qt)
ItemObserverPointer item1 = ...;
ItemObserverPointer item2 = ...;
DeleteItem(item1);  // это удалит item1 и item2
DeleteItem(item2);  // а это вылет

И вот я типа могу заюзать shared_ptr и хранить в кэше шареды - всё чики-пуки... Кроме тех мест где таки явно зовется delete object с целью удалить их из кэша

Тогда кеш становится одним из владельцев, а значит хранимые там объекты живут forever. Городил велик на эту тему (в "кладовой")

[Авварон]

но почему оно должно рухнуть?

Пусть выбраны 2 айтема и первый - парент второго

Код

C++ (Qt)
ItemObserverPointer item1 = ...;
ItemObserverPointer item2 = ...;
DeleteItem(item1);  // это удалит item1 и item2
DeleteItem(item2);  // а это вылет

То есть вы сперва придумали функцию "как бы поудобнее" а потом нашли способ выстрелить себе ей в ногу? Ну что ж, поздравляю, у меня такой функции нет и я в ногу не стреляю. Совпадение? Не думаю.

Шутки шутки, никто не обещал что умные указатели не дадут выстрелить в ногу. Так-то можно сделать delete smart_ptr.get() и долго удивляться как же так вышло.
В вашем примере достаточно очевидно что проблема во временном хранилище - списке выделенных элементов.
По сути, вы итерируетесь по списку меняя его в процессе. Ну так фигли, хотите я вам такое же на std::vectorе напишу? Тоже будет падать. И будет падать независимо от того, вумные указатели в векторе или обычные.