goto vs break & continue , Кроха-сын пришел к отцу и спросила кроха: "goto - это хорошо?"

Цитата Wound @ 20.07.20, 20:30

Если у тебя вызов сишной функции возвращает код возврата - кто тебе мешает кидать исключение в конструкторе RAII обертки?

Не разобравшись - советуешь!
При чем тут вообще исключения? Вопрос в другом. В случае успешного прохождения полной цепочки инициализаций - ничего освобождать не нужно. После выхода из процедуры отдельные ресурсы должны оставаться валидными. Но, если не вся цепочка пройдена - вот тогда все ресурсы, захваченные до ошибки, должны быть освобождены.

Цитата D_KEY @ 20.07.20, 20:49

Плохая практика, не надо так делать. Прочитай про инварианты класса.

Цитата

Инвариа́нт — это свойство некоторого класса (множества) математических объектов, остающееся неизменным при преобразованиях определённого типа.

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

1) Ожидает
2) В состоянии соединения
3) В состоянии ошибки
4) В состоянии передачи

Причем возможные переходы между состояниями заранее предопределены. В этом случае инвариантность не нарушается.

Добавлено 20.07.20, 21:49

Цитата Qraizer @ 20.07.20, 21:41

Ну так это не RAII

Получается - скоуп экзиты штоле тогда?

Цитата Qraizer @ 18.07.20, 18:16

Быстренько налобал правильный тест.

Скинь, плс, исходники потестить.

Цитата JoeUser @ 20.07.20, 21:45

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

Это решается путём передачи владения ресурсом полю или передачи его(владения) наружу.

Добавлено 21.07.20, 00:18

Цитата JoeUser @ 20.07.20, 21:45

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

1) Ожидает
2) В состоянии соединения
3) В состоянии ошибки
4) В состоянии передачи

Причем возможные переходы между состояниями заранее предопределены. В этом случае инвариантность не нарушается.

Допустим. ИМХО, тогда лучше воспользоваться паттерном state и управление ресурсами в этом случае будет примерно такое, как я описал. Но дело твое. В методе load в случае успеха передать владение от локального объекта полю все равно никто не мешает. Если не понятно, то могу код привести чуть позже.

Добавлено 21.07.20, 00:31
https://godbolt.org/z/zbhGr5

#include <iostream>
#include <memory>
 
struct my_initializer {
    my_initializer()
    {
        std::cout << "init" << std::endl;
    }
    ~my_initializer()
    {
        std::cout << "deinit" << std::endl;
    }
};
 
struct my_struct {
    void load()
    {
        auto init = std::make_unique<my_initializer>();
        // ...
        bool success = true;
 
        // ...
 
        if (success) {
           init_.swap(init);
        }
    }
 
private:
    std::unique_ptr<my_initializer> init_;
};
 
int main()
{
    my_struct a;
    {
        std::cout << "enter" << std::endl;
        a.load();
        std::cout << "exit" << std::endl;
    }
}

Поигравшись с переменной success сможешь понять, в чем суть.

Сообщение отредактировано: D_KEY - 21.07.20, 00:32

Не, это уже чересчур! Еще и локальные переменные заводить дополнительно, потом их обменивать ... ради чего? Попахивает антипаттерном.

Это передача владения ресурсом.
Код простой, никаких дурацких флагов, if, goto, понятно, кто ресурсом владеет. Не надо даже деструктор у my_struct писать. Да даже guard и лямбд.
Просто сравни с твоей лапшой.

По поводу локальных объектов и обмена - гугли про безопасность исключений (на самом деле это касается любой обработки ошибок, если подумать). Если сам не найдешь толкового материала, я позже могу раскрыть тему.

И у меня нет "дурацких флагов", есть описатели состояния. И goto уже нет. Говоришь нет дурацких if'ов:

if (success) {
  init_.swap(init);
});

По-моему уже пришли к вопросу бутерброда, с какой стороны правильно намазывать масло. В моем варианте - обеспечить неудаление нужного. В твоем варианте - обеспечить передачу владения. Что по сути звучит. и то, и то одинаково: не потерять лишнее. Но раз, ты нагородил еще более моего, обеспечивая, по сути, тоже самое - вот я и говорю. Антипаттерном попахивает.

Цитата JoeUser @ 20.07.20, 21:45

Не разобравшись - советуешь!
При чем тут вообще исключения? Вопрос в другом. В случае успешного прохождения полной цепочки инициализаций - ничего освобождать не нужно. После выхода из процедуры отдельные ресурсы должны оставаться валидными. Но, если не вся цепочка пройдена - вот тогда все ресурсы, захваченные до ошибки, должны быть освобождены.

Почитай как работает система исключений в С++ при конструировании объекта.
На ка тебе пример еще в догонку, расскажи пожалуйста что тут работает не так, как ты описал? Очень интересно послушать.
https://ideone.com/SI8Kz6

#include <iostream>
using namespace std;
 
struct Resource
{
    Resource(int id)
    {
        
        m_id = id;
        if(id == 3)
        {
            throw "Error during initialize: id can't be 3";
        }
        std::cout << "initialize resource with id: " << id << std::endl;
 
    }
    
    ~Resource()
    {
        std::cout << "Destroy Resource with id: " << m_id << std::endl;
    }
    
private:
    int m_id;
};
 
struct ResourceWrapper
{
    ResourceWrapper(int id1, int id2, int id3, int id4, int id5)
    : m_rc1(id1),
      m_rc2(id2),
      m_rc3(id3),
      m_rc4(id4),
      m_rc5(id5)
    {
    }
    
private:
   Resource m_rc1;
   Resource m_rc2;
   Resource m_rc3;
   Resource m_rc4;
   Resource m_rc5;
};
 
int main()
{
    
    {
        std::cout << "without errors:" << std::endl;
        ResourceWrapper rc(5,6,7,8,9);
    }
    
    std::cout << "==========================" << std::endl;
    try
    {
        std::cout << "with errors:" << std::endl;
        ResourceWrapper rc(1,2,3,4,5);
    }
    catch(const char* error)
    {
        std::cout << "Error: " << error << std::endl;
    }
    // your code goes here
    return 0;
}

Сообщение отредактировано: Wound - 21.07.20, 07:36

Цитата JoeUser @ 21.07.20, 06:26

Говоришь нет дурацких if'ов

Да, нет. Конкретно этот if не дурацкий, кроме того, его вполне может не быть, он там лишь для примера, чтоб ты мог управлять успешным/не успешным завершением.

Цитата

В твоем варианте - обеспечить передачу владения.

В чем тут трудность или проблема-то? У ресурсов должны быть владельцы - это хороший подход, очень многие вещи упрощает.

Цитата

Но раз, ты нагородил еще более моего

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

Но кто я такой, чтобы мешать тебе продолжать писать говнокод?

Добавлено 21.07.20, 07:36

Цитата JoeUser @ 21.07.20, 06:26

В моем варианте - обеспечить неудаление нужного. В твоем варианте - обеспечить передачу владения. Что по сути звучит. и то, и то одинаково: не потерять лишнее.

Я не понимаю тебя, если честно.

Давай начнем с простого.
Вот это, на твой взгляд, одинаково "звучит":

int * p = new 10;
 
// ...
 
delete p;

И

auto p = std::make_unique<int>(10);
 
// ...

?

Цитата D_KEY @ 21.07.20, 02:35

Это передача владения ресурсом.

Чем то похоже на идиому copy-and-swap https://ru.wikipedia.org/wiki/Copy-and-swap

Цитата Wound @ 21.07.20, 07:46

Чем то похоже на идиому copy-and-swap

Ну да. Что бы ни произошло в методе load, это не затронет состояние, пока работа не будет завершена. Сначала делаем все операции, которые могут привести к ошибке, потом меняем состояние (тут уже ошибок быть не может).

Хотя конкретно тот мой код просто пример, а не реальный код.

Сообщение отредактировано: D_KEY - 21.07.20, 08:02

Цитата Wound @ 21.07.20, 07:27

Почитай как работает система исключений в С++ при конструировании объекта.

а то я не знаю?

Цитата Wound @ 21.07.20, 07:27

На ка тебе пример еще в догонку, расскажи пожалуйста что тут работает не так, как ты описал? Очень интересно послушать.

1) Ты собираешь независимые ресурсы в свой wrapper (хотя логичнее назвать holder). У меня они зависимые. Т.е. результат одного передается в инициализацию другого. Но и это не главное.
2) Один (или несколько, пока один) из ресурсов у меня "временный" и его хранить не нужно. Т.е. r1,rc2, rc3, rc4 проинициализировали, все rc3 уже не нужен, удалили, потом - rc5, rc6
3) Ну и самое наверное важное - ты решил в свой wrapper запилить в конструкторе больше чем нужно. Т.е., как объект создается - так он сразу бежит логиниться. Такое мне не нужно!

Но я понимаю, к чему ты клонишь - раздербанить мой sftp_class в целую "инфраструктуру", типа:

winsock_class - WSAStartup /WSACleanup();
libssh2_class - libssh2_init / libssh2_exit
resolv_slass - getaddrinfo / freeaddrinfo
socket_class - socket / closesocket
connect_class - connect / shutdown
libssh2_session_init_class - libssh2_session_init / libssh2_session_free
libssh2_sftp_sesstion_class - libssh2_sftp_init / libssh2_sftp_shutdown

Потом мне придется писать такую же портянку исключений, чтобы понять кто бросил, и можно ли продолжить работу ( допустим сделать еще 3 попытки соединения)

В результате вся эта "правильность" вылезает боком в увеличении кода и сложности взаимодействий. Пусть это четырежды правильнее - но самому находить работу, ради чистоты рассы, имхо - тупо и глупо! Лучше уж костыли, чем велосипеды с мотором от трактора.

Добавлено 21.07.20, 10:11
ADD: Нет, ну я понимаю, когда система сложная, когда сложно отследить все взаимодействия - тогда нет вопросов. Но при таком линейном выполнении городить огород, просто, чтобы "соответствовало идеологии", не считаю это разумным.

Цитата JoeUser @ 21.07.20, 10:03

а то я не знаю?

Судя по тому что ты мне ответил - возникает ощущение, что не знаешь.

Цитата JoeUser @ 21.07.20, 10:03

1) Ты собираешь независимые ресурсы в свой wrapper (хотя логичнее назвать holder). У меня они зависимые. Т.е. результат одного передается в инициализацию другого. Но и это не главное.
2) Один (или несколько, пока один) из ресурсов у меня "временный" и его хранить не нужно. Т.е. r1,rc2, rc3, rc4 проинициализировали, все rc3 уже не нужен, удалили, потом - rc5, rc6

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

Цитата JoeUser @ 21.07.20, 10:03

3) Ну и самое наверное важное - ты решил в свой wrapper запилить в конструкторе больше чем нужно. Т.е., как объект создается - так он сразу бежит логиниться. Такое мне не нужно!

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

Цитата JoeUser @ 21.07.20, 10:03

Но я понимаю, к чему ты клонишь - раздербанить мой sftp_class в целую "инфраструктуру", типа:

winsock_class - WSAStartup /WSACleanup();
libssh2_class - libssh2_init / libssh2_exit
resolv_slass - getaddrinfo / freeaddrinfo
socket_class - socket / closesocket
connect_class - connect / shutdown
libssh2_session_init_class - libssh2_session_init / libssh2_session_free
libssh2_sftp_sesstion_class - libssh2_sftp_init / libssh2_sftp_shutdown

Это не я к этому клоню - это принцип идиомы RAII. Один ресурс - одна обертка. В твоем случае у тебя тут выходит вот столько вот оберток, сколько ты тут написал. opn/close - один RAII класс/ну или смарт поинтер, в заивисимости от сложности реализации инициализации/уничтожения. Это вроде как стандартный подход

Цитата JoeUser @ 21.07.20, 10:03

Потом мне придется писать такую же портянку исключений, чтобы понять кто бросил, и можно ли продолжить работу ( допустим сделать еще 3 попытки соединения)

Данные нужно правильно структурировать, 3 попытки соединения ты можешь сделать в RAII обертке. А дальше, если у тебя соединение не прошло - кидать исключение/писать в лог, потому что на сколько я понимаю - дальнейшая работы с ресурсами лишена всякого смысла, соединение то не установлено. Дальше ты это исключение ловишь - где тебе нужно, пишешь его в лог/показываешь юзеру, и совершенно не паришься по поводу того - нужно тебе там что то освободить или нет.

Цитата JoeUser @ 21.07.20, 10:03

В результате вся эта "правильность" вылезает боком в увеличении кода и сложности взаимодействий. Пусть это четырежды правильнее - но самому находить работу, ради чистоты рассы, имхо - тупо и глупо! Лучше уж костыли, чем велосипеды с мотором от трактора.

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

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

Добавлено 21.07.20, 10:22
Но я бы тебе советовал перейти на другой ЯП, желательно с GC, тебе не нужен С++, ты просто не юзаешь его фишки как бы

Добавлено 21.07.20, 10:25
Да хоть тот же Си юзай, он поди быстрее С++ и меньше памяти рантайм жрать будет.

Сообщение отредактировано: Wound - 21.07.20, 10:25

Цитата Wound @ 21.07.20, 10:22

Судя по тому что ты мне ответил - возникает ощущение, что не знаешь.

Судя по тому, как ты судишь - ты не умеешь судить. 1-1 Ланна, Киля, спасибо за участие, я тебя услышал!
D_KEY, и тебе спасибо

Вот посмотри как твоя задача(ну или подобная) решается с помощью RAII -> https://habr.com/ru/company/ispsystem/blog/430488/

Добавлено 21.07.20, 10:51
В итоге весь твой код в функции размазывается на классы - это основная библиотека как бы выходит. А дальше ты уже с ними работаешь где тебе нужно.

Добавлено 21.07.20, 10:54
В итоге ты как бы разделяешь свой код на слои. Когда ты начинаешь юзать свои вот эти обертки - тебе уже совершенно не интересно что в них происходит, и ты сосредотачиваешься на алгоритме работы. Если где то произошла ошибка - она легко детектится и локализуется.

А вот то что ты там налабал - это больше Сишный подход, и то там наверное свои паттерны имеются чтоб упростить подобную задачу.

Добавлено 21.07.20, 11:05
Вот тебе еще пример RAII оберток для ssh -> https://github.com/rubdos/libssh2pp/blob/master/libssh2.hpp

Цитата Wound @ 21.07.20, 10:49

Вот тебе еще пример RAII оберток для ssh

Видел это. тут, кстати, недообернули)))

#ifdef WIN32
  WSADATA wsadata;
  WSAStartup(MAKEWORD(2,0), &wsadata); <----------------это один ресурс в классе Session, и нужно чекать возврат
#endif
   // Initialize libssh2 on a thread safe manner, count the session instances.
   libssh2::__libshh2_session_count_mutex.lock();
...
...
...
  this->_sess = libssh2_session_init(); <------------- а это уже второй ресурс в классе Session