Obsługa błędów w C++ a drabinka if'ów

0
Sniffer sniffer;
if (!sniffer.setNetworkingInterface()) {
	std::cerr << "Error with setting sniffer's networking \
			interface\r\n";
	return -2;
}
if (!sniffer.bindSnifferSocket()) {
	std::cerr << "Fail with binding sniffer\'s socket\r\n";
	return -3;
}

Czy w takim przypadku użycie dużej ilości if'ów jest dopuszczalne? Trochę kłuje mnie to w oczy, a ilość tego typu if'ów może się powiększać wraz z ilością wywoływanych funkcji, bo błędy muszę jakoś obsłużyć. Zostawić tak jak jest czy pasowałoby podejść do obsługi błędów nieco inaczej?

Gdyby potrzebna była większa ilość kodu to tu jest link do projektu -> https://github.com/shizzeer/NetObserver
Kodu jest mało, bo dopiero zaczynam tworzyć ten projekt więc nie ma dużo do analizy.

2

Jeśli to jest funkcja jakiegoś interfejsu to można ją traktować jak "plik nagłówkowy" więc nie widzę w tym nic złego.
Lepiej pewnie to pogrupować i coś odrobinę uogólnić tak aby komunikaty i if'y nie były wprost w tym miejscu.
Może zrobić jakąś ogólną klasę obsługi błędów żeby w kodzie to ładniej wyglądało i wywoływać ją z miejsc gdzie błąd wystąpił ...
Jednak jeśli faktycznie interfejs obsługuje 100 funkcji/komunikatów/błędów to nie widzę nic złego w tym, że po kolei będzie wywołane 100 podobnych fragmentów kodu.
Nie poddawajmy się terrorowi, że każda funkcja musi mieć max 20 linii. Oczywiście jestem zwolennikiem krótkich funkcji ale czasem jest zwyczajnie czytelniej i wygodniej zrobić długą w szczególności jeśli taka funkcja służy jedynie do zwykłego "przewalania danych z lewa na prawo".

Pytanie też ile tego będzie ...

2

Czy Sniffer to twój kod i możesz go modyfikować? Ta drabinka ifów i tak zostanie w jakiś sposób zaimplementowana, pytanie to w jakiej formie. Kod który wrzuciłeś to typowy sposób pisania znany w języku C. Najbardziej idiomatycznym sposobem obecnie używanym w C++ są wyjątki. Innym sposobem idącym w zupełnie inną stronę są typy opakowujące wartość zwracaną znane z języków funkcyjnych. Google ostatnio wrzuciło swoją bibliotekę wspierającą programowanie w tym stylu https://abseil.io/docs/cpp/guides/status

2

Właśnie też chciałem pisać o wyjątkach ale pytający przesłał taki klasyczny kod z przekazywaniem kodu błędu.
Co do wyjątków to jestem zwolennikiem prostej zasady. Jeśli błąd ma wędrować więcej niż 2 poziomy wyżej od miejsca powstania to sugeruję użyć wyjątki. Przekazywanie błędów w "głębokim kodzie" jako wyniku funkcji to droga do piekła ... Owszem czasem jest to szybsze i bardziej precyzyjne ale w sumie po to też są wyjątki, które przy okazji ogarną nam jeszcze inne nieprzewidziane sytuacje.

4

IMO w poście podałeś za mały kontekst, żeby coś stwierdzić. Z Twojego main na githubie wnioskuję, że te metody muszą zostać wywołane, żeby obiekt sniffera miał sens i był używalny. Ja kieruję się prostą zasadą - jeżeli masz obiekt, to obiekt ma być używalny i już. Nie ma być tak, że potem jeszcze trzeba wywoływać pomocnicze metody, żeby doinicializować obiekt. Konstruktor i koniec.

Nigdy nie byłem zwolennikiem rzucania wyjątków w konstruktorze. Szczególnie jeżeli mam do czynienia z nie-RAII typami, jak te z WinAPI, których używasz. W takich wypadkach wolałem zrobić metodę wytwórczą czy jakąś fabrykę, która zajmuje się inicializacją i zwróci mi albo obiekt, albo info, że się nie powiodło (np. std::option<Sniffer>). Mimo wszystko nie chcę mówić, że takie podejście jest jedyne prawilne. Jeżeli chcesz rzucać wyjątkiem w ctorze, to spoko. Zadbaj tylko o posprzątanie przed rzuceniem, bo pamiętaj, że destruktor dla tej instancji się nie wywoła

Podsumowując, w ja osobiście w Twoim przypadku przeniósłbym wszystko do metody wytwórczej, która zwraca std::option. Wywoałałaby ona wszystkie inicializacje z WinAPI, a potem przekazała do ctora Sniffera ogarnięte już wartości wsaData, snifferSock i netInterface.

Jeżeli chodziło Ci tylko i wyłącznie o odpowiedź czy taka obsługa błędów jest ok, to moja odpowiedź jest taka, że jak tak trzeba, to tak trzeba i tyle. Jak jest takie AIP, to tego nie przeskoczysz. Jak możesz API zmienić, to bym postarał się to zmienić.

(Jeżeli w którymś momencie przyjąłem błędne założenia, to doprecyzuj proszę.)

1
katakrowa napisał(a):

Właśnie też chciałem pisać o wyjątkach ale pytający przesłał taki klasyczny kod z przekazywaniem kodu błędu.

Co do wyjątków to jestem zwolennikiem prostej zasady. Jeśli błąd ma wędrować więcej niż 2 poziomy wyżej od miejsca powstania to sugeruję użyć wyjątki. Przekazywanie błędów w "głębokim kodzie" jako wyniku funkcji to droga do piekła ... Owszem czasem jest to szybsze i bardziej precyzyjne ale w sumie po to też są wyjątki, które przy okazji ogarną nam jeszcze inne nieprzewidziane sytuacje.

Popieram to myślenie.

Jestem dużo za, a mało przeciw.
Przeciw to mowa o nadużyciu wyjątków do typowego sterowania, dyskusja znana w OOP
Za są wszystkie inne argumenty, na czele z tym, że zaprojektowane kody błędów ZAWSZE mają braki, coś nie jest zaprojektowane, a wyjątki łapią i nieplanowane sytuacji

0
katakrowa napisał(a):

Jeśli to jest funkcja jakiegoś interfejsu to można ją traktować jak "plik nagłówkowy" więc nie widzę w tym nic złego.

Ta funkcja, której fragment wkleiłem to main. Jeśli szedłbym dalej w zwracanie kodu błędów to przewiduję, że takich if'ów mogłoby być całkiem sporo występujących po sobie.

slsy napisał(a):

Czy Sniffer to twój kod i możesz go modyfikować? (...) Kod który wrzuciłeś to typowy sposób pisania znany w języku C. Najbardziej idiomatycznym sposobem obecnie używanym w C++ są wyjątki.

Sniffer to mój kod - wkleiłem link do githuba w poście na dole. Co do wyjątków to wiem, że istnieją i użyłem ich do obsługi błędów w konstruktorze obiektu sniffer, ale tylko dlatego, że przeczytałem na SO, iż wyjątki można spokojnie stosować do obsługi błędów w konstruktorach, bo one nie zwracają wartości. Niedawno napisałem taki temat odnośnie stosowania if'ów i wyjątków -> https://4programmers.net/Forum/C_i_C++/343224-wyjatki_a_ify_podczas_obslugi_bledow?p=1701217#id1701217 Z niego nauczyłem się, że wszystko co mogę obsłużyć i na co mam wpływ podczas implementacji algorytmu obsługuję bez użycia wyjątków, a wyjątki używam do obsługi błędów, na które wpływu nie mam. Na przykład jakiś bug w funkcji systemowej gdzieś wewnątrz spowodowałby, że OS nie zwróciłby mi uchwytu do pliku, z którego chciałem skorzystać, a plik w systemie istnieje. W takim przypadku użyłbym wyjątku, a tutaj użyłem if'ów zgodnie z nauką, którą z tamtego tematu wyniosłem. Nie wiem czy dobrze robię czy nie w końcu - wciąż się uczę. :D

katakrowa napisał(a):

(...) Jeśli błąd ma wędrować więcej niż 2 poziomy wyżej od miejsca powstania to sugeruję użyć wyjątki. Przekazywanie błędów w "głębokim kodzie" jako wyniku funkcji to droga do piekła ... Owszem czasem jest to szybsze i bardziej precyzyjne ale w sumie po to też są wyjątki, które przy okazji ogarną nam jeszcze inne nieprzewidziane sytuacje.

Czyli jeśli dojdę z wywołaniami funkcji gdzieś głębiej mogę użyć wyjątku do obsługi błędu zamiast schodzić w dół stosu, żeby było wygodniej? Dobrze rozumiem?

stryku napisał(a):

(...) Ja kieruję się prostą zasadą - jeżeli masz obiekt, to obiekt ma być używalny i już. Nie ma być tak, że potem jeszcze trzeba wywoływać pomocnicze metody, żeby doinicializować obiekt. Konstruktor i koniec.

Też o tym myślałem, ale mnie zawsze kłują w oczy długie funkcje. Wiem, że może to przesada i nie powinienem się tym nadto przejmować, ale jakoś mam alergię na takie działanie. Myślałem też o tym, że obiekt "skonstruować" tak, żeby był w pełni używalny, ale bałem się o rozmiar konstruktora. Czyli uważasz, że rozpiętość kodu konstruktora jest mniej ważna, a bardziej ważna jest pełna używalność obiektu po jego skonstruowaniu? Jeśli mogę taką zasadę stosować nie patrząc zbytnio na rozpiętość tego kodu to świetnie. :) Tzn. oczywiście muszę dopowiedzieć, że ten kod wewnątrz konstruktora i tak dzieliłbym na metody, żeby było możliwie krótko.

Nigdy nie byłem zwolennikiem rzucania wyjątków w konstruktorze. Szczególnie jeżeli mam do czynienia z nie-RAII typami, jak te z WinAPI, których używasz. W takich wypadkach wolałem zrobić metodę
wytwórczą czy jakąś fabrykę, która zajmuje się inicializacją i zwróci mi albo obiekt, albo info, że się nie powiodło (...)

O wyjątkach rzucanych w konstruktorach przeczytałem na SO. Szukałem info na temat tego jak obsłużyć błąd w konstruktorze, bo konstruktor nie zwraca żadnej wartości. Właśnie z tego powodu radzili, żeby użyć wyjątki.

W takich wypadkach wolałem zrobić metodę wytwórczą czy jakąś fabrykę, która zajmuje się inicializacją i zwróci mi albo obiekt, albo info, że się nie powiodło (np. std::option<Sniffer>).

O std::option nie wiedziałem, dzięki za podrzucenie tematu. :) Ten projekt służy mi przede wszystkim do nauki C++ a finalnie być może przyda mi się do inżynierii wstecznej. Wiadomo, że jest Wireshark ze snifferów, ale jest bardzo rozbudowany - jak dla mnie aż za bardzo, a poza tym miło jest korzystać z projektów autorskich. :D

Podsumowując, w ja osobiście w Twoim przypadku przeniósłbym wszystko do metody wytwórczej, która zwraca std::option. Wywoałałaby ona wszystkie inicializacje z WinAPI, a potem przekazała do ctora Sniffera ogarnięte już wartości wsaData, snifferSock i netInterface.

I to jest bardzo ciekawe podejście, na które bym nie wpadł, bo nie wiedziałem nawet o istnieniu std::option. Postaram się coś podobnego zaimplementować, bo brzmi dobrze. :)

Dziękuję wszystkim za pomoc. Jeszcze @katakrowa jakbyś mógł mi wyjaśnić czy dobrze zrozumiałem to wędrowanie 2 poziomy wyżej od miejsca powstania błędu to byłbym wdzięczny.

2

Analiza SO wyjątków w konstruktorze na jaką się powołujesz, wydaje się spłycona.
Powiedzieć że wyjątek to przekazana informacja to jest prawda, ale częściowa. Nie mniej ważne jest zerwanie biegu programu.

W językach Java i C# jest to o tyle prostsze, że 100% obiektów tworzonych jest przez new, i nieprawidłowy obiekt nigdy nie powstanie (co jest bardzo cenne).
W C++ jest ten nieszczęsny model pamięci "auto", czyli na stosie (oraz static). Konstruktor się "zerwał" wyjątkiem, ale obiekt nadal istnieje.
Gdyby zapewnić dyscyplinę braku "auto" jak w C#/Java by działało to super.

Tu zadam pytanie, bo nie wiem, czy w nowszych standardach C++ można zablokować "auto" dla jakiejś klasy?

@katakrowa miał prawie na pewno na myśli, że przekazywanie kodu (opisu) błędu przez wiele warstw jest
a) żmudne
b) błedogenne
c) dziurawe, nie da się tego zrobić naprawdę dobrze
Liczna 2 to taka licentia poetica

0

Więc kod ... piszę ... i kurcze możesz mieć rację ...

// ConsoleApplication12.cpp : This file contains the 'main' function. Program execution begins and ends there.
//

#include <iostream>
#include <exception>

class Cokiowiek
{
    int a, b, c;
public:
    Cokiowiek() {
        a = 1;
        //        if (pogoda == deszcz)
        {
            throw new std::exception("Jak mi się nie chce");
        }
        b = 5;
        c = 2;
    };
    ~Cokiowiek() {
        if (b)
            a = c;
    };
};

int main()
{
    if (1)
    {
        Cokiowiek cosik;
        int c = 99;
    }
    std::cout << "Hello World!\n";
}


Wychodzi mi, ze zerwany będzie (przynajmniej) blok zawierający, czyli nieco inaczej niz w Javie / C#, ale efektywnie użycie obiektu nie zajdzie.
Mam jeszcze jakies mgliste przypuszczenie, że intensywnie używając try catch można zaburzyć taki (w sumie prawidłowy) przebieg ... ale nie umiem podać przykładu

EDIT: dodaję destruktor i ... piknie, będzie jeździł na złym stanie obiektu ...

0

Mój obecny kod ma wprowadzony niedeterminizm (kopała nas optymalizacja).
Skonfundowany jestem co wyrabia MS VS 2019
Debug wykonuje kod za throw

#include <iostream>
#include <exception>

class Cokiowiek
{
public:
    int a, b, c;
    Cokiowiek() {
        a = 1;
        if (getchar()=='c')
        {
            std::cout << "Wyjatek\n";
            throw new std::exception("Jak mi się nie chce");
        }
        b = 5;
        c = 2;
    };
    ~Cokiowiek() {
        std::cout << "\nDestruktor! "<<a<<' '<<b<<' '<<c;
        if (b)
            a = c;
    };
};

int main()
{
    std::cout << "Hello World!\n";
    try
    {
        Cokiowiek cosik;
        std::cout << "Wartosci a b c " << cosik.a << ' ' << cosik.b << ' ' << cosik.c;
       
        int c = 99;
    }
    catch (std::exception e)
    {
        std::cout << "Exception " << e.what();
    }

    std::cout << "\nKoniec !\n";
    getchar();
}



Wynik Debug
Hello World!
c
Wyjatek
Wartosci a b c 1 5 2
Destruktor! 1 5 2

Release
Hello World!
c
Wyjatek
Unhandled exception thrown: read access violation. (w kółko)
this was 0x13247698.

Q ja tego bez wódki nie zrozumiem

1

@Shizzer: nie ma reguły, ale osobiście uważam, że dobrą praktyką jest korzystanie z RAII, czyli krótko mówiąc - jeżeli na skutek jakiegoś wywołania w danym bloku wszystko staje się bezużyteczne, to opuszczasz scope i wszystko sprząta się samo. Czy przez wyjątek czy przez returna - sprawa otwarta.

Natomiast mam jedno takie szersze przemyślenie: moim zdaniem piszesz teraz bardziej w C z klasami niż C++; nic w tym złego, ale zastanów się czego potrzebujesz.
Bo skoro już w tym C++ piszesz, to może warto by było np. zapakować calle do WSAStartup/WSACleanup w osobny obiekt i korzystając z RAII wywołać je automatycznie? Teraz tego kodu jest tam mało, ale taka ifologia zrobi się wkrótce mocno nieczytelna.

0
alagner napisał(a):

Natomiast mam jedno takie szersze przemyślenie: moim zdaniem piszesz teraz bardziej w C z klasami niż C++; nic w tym złego, ale zastanów się czego potrzebujesz.

Jeśli mój kod jest blisko kodu w C to chętnie przyjmę wszystkie propozycję jak go poprawić, żeby blisko C nie był. Tzn. ja niejako "wywodzę się" z C i dopiero wchodzę w świat C++'a toteż zależy mi na tym, aby mój kod od C jednak odbiegał. Chciałbym w swoich projektach zachować low-level na tyle, żeby na przykład przy tworzeniu sniffera nie korzystać z gotowych bibliotek, a działać na socketach natomiast opis abstrakcji powinien być jak najbardziej zrozumiały, a zatem korzystający jak najbliżej z dobrodziejstw C++'a.

Bo skoro już w tym C++ piszesz, to może warto by było np. zapakować calle do WSAStartup/WSACleanup w osobny obiekt i korzystając z RAII wywołać je automatycznie? Teraz tego kodu jest tam mało, ale taka ifologia zrobi się wkrótce mocno nieczytelna.

Mógłbyś mi napisać jak to RAII miałoby dokładnie wyglądać posługując się jakimś krótkim kodem przykładowym? Co do implementacji obiektu sniffer postarałem się, aby socket i te struktury zainicjalizowane przez WSA były zwalniane gdy obiekt sniffer opuści swój range, czyli myślałem, że użyłem tu RAII. Jedynie wyjątek tam powoduje, że instancja obiektu się nie utworzy, ale wtedy zasoby też są odpowiednio zwalniane.

1

Popraw ten kod, bo połowa rzeczy tutaj nie działa. Raz, że std::exception nie ma takiego konstruktora. Dwa, że rzucasz std::exception* (bo operator new), a łapiesz std::exception slsy 13 minut temu

NAJGORSZE jest to, że działa.

zmieniłem zgodnie z sugestią catcha na

catch (std::exception * e)

Działa tak, jakby się spodziewał.

Jestem patologicznie zdumiony, jak inny formalnie catch (największe, co by się spodziewać, to nie dopasowanie exception, i tak było) wysada w powietrze resztę kodu. Właśnie sobie zmieniam w Debug i Release tą gwiazdkę w catch'u w te i wewte, i wracają patologie.

Ostatni raz taką rozpacz kompilatora widziałem na kilka tys liniowym pliku w BC++B, gdy strzał był na typie zaszytym w kompilator (TTime czy coś w tym stylu, kalka z Delphi). Widać było, w jak strasznym pocie czoła trzy dni przed upływem deadline patchowali zdrowy parser kompatybilnością z Delphi.

Nie sądziłem, że coś podobnego kiedyś zobaczę.
Do q nędzy, czy ja coś zabójczego dla kompilatorów tu wymyśliłem?

0

no właśnie też skompilowałem... i nadal nie wiem z czym masz problem. To nie jest może (nawet na pewno) dobre, że da się rzucić wszystko...ale nadal nie widzę opisywanych przez Ciebie wcześniej problemów. alagner dziś, 01:32

"wcześniej" ... ?

To może w punktach... Wcześniej postawiłem tezę, ze obiekt w klasie pamięci auto (wiem, ze słowo kluczowe się z tym kłoci - jak obecnie się mówi o modelu pamięci 'auto'?) ... że da się używać obiektu nie w pełni skonstruowanego (konstrukcja przerwana wyjątkiem, ale obszar pamięci reprezentujący obiekt istnieje).

Potem spokorniałem, ciągle na etapie analizy teoretycznej, bo skoro wyjątek zerwał wykonanie bloku, to nie ma o czym mówić.

ALE POTEM pomyślałem, zaczaję się np na destruktor, i to już było z realnym środowiskiem.

Zostawmy na uboczu dyskusji "prawidłowy catch", on zachowuje się sensownie.

grube jaja są "nieprawidłowym catchem" w tym konkretnym środowisku VS 16.7.4 i 16.7.5 (w międzyczasie upgradowałem)
Przyznasz teoretycznie, ze nieprawidłowy catch ma skutkować zaledwie brakiem obsługi wyjątku - ale bez zmiany działania kodu głównego?
Wszystko co mówię od tej pory dotyczy throw new exception 1) i catch exception

Dziś już zrobiłem to z większym namysłem. Problem ma inną barwę podczas debugowania (krokowania), a inną podczas wykonania debugowej binarki w konsoli

Debug, dobry catch (przebieg jest zgodny z oczekiwaniem)
Hello World!
c
Wyjatek
Exception
Koniec !

Debug, zły catch, pod debugerem
Hello World!
c
Wyjatek
Wartosci a b c 1 5 2
(przy czym tutaj następuje wejście w destruktor, ale jego wydruk już nie następuje, wylot)

UPDATE! po wspominanych nieprawidłowych buildach jest takie wyjście
Hello World!
c
Wyjatek
Wartosci a b c 1 5 2
Destruktor! 1 5 2
Koniec !

Wersja Debug, skompilowany program w konsoli, w żadnej z trzech możliwości kliknięcia nie zachodzi wypisanie Exception
screenshot-20201011100452.png

Mam mocne wrażenie, ze temat debugowania chłopaki robili trzy dni przed deadline

Generalnie ja chyba jestem toksycznym C++ programersem, np używam białego tła, a prawdziwi programiści grafitowego
Dziś wielokrotnie blokowałem sobie działanie Visual Studio w przeróżne sposoby, np z wielkim "nic" na wykonaniu

'ConsoleApplication12.exe' (Win32): Loaded 'C:\Users\Jacek\source\repos\ConsoleApplication12\x64\Debug\ConsoleApplication12.exe'. Symbols loaded.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\ntdll.dll'.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\kernel32.dll'.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\KernelBase.dll'.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\msvcp140d.dll'.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\vcruntime140d.dll'.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\vcruntime140_1d.dll'.
'ConsoleApplication12.exe' (Win32): Loaded 'C:\Windows\System32\ucrtbased.dll'.
The thread 0x2a34 has exited with code 0 (0x0).
The thread 0x1e3c has exited with code 0 (0x0).
The thread 0x304c has exited with code 0 (0x0).

albo taki build

1>------ Rebuild All started: Project: ConsoleApplication12, Configuration: Debug Win32 ------
1>A task was canceled.
1>A task was canceled.
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========
(ale errorów zero)

  1. nigdy nie było zakazane, najwyżej negatywnie komentowane
0

@AnyKtokolwiek: znalazłeś bug w MSVC najpewniej, nie pierwszy zresztą, testowałem Twój kod pod clangiem na Macu, działa dobrze. Ew. to "ficzer"/rozrzeszenie, bo np. std::exception nie ma konstruktora przyjmującego tekst, przynajmniej wg https://en.cppreference.com/w/cpp/error/exception/exception, a skoro Ci się kompiluje to znaczy że masz włączone coś ponad standardowe C++
Ja rozumiem, że z praktycznego punktu widzenia to jest problem kojarzony z językiem, bo MSVC jest mocno popularny, w rzeczywistości jednak leży on gdzie indziej, zwłaszcza, że MSVC na dzień dobry włączone te wszystkie swoje dziwactwa i to naprawdę nie ułatwia pisania dobrego, przenośnego kodu.

@kq jesteś w stanie to skompilować na MSVC w jakimś mocno pedantycznym trybie?

1 użytkowników online, w tym zalogowanych: 0, gości: 1