C++ Code Reinheit

Question

Ich arbeite in C++ Umgebung und:
a) Wir sind verboten Ausnahmen verwenden
b) Es ist application/Datenserver-Code, der viele Anfragen von verschiedenen Arten

I wertet haben einfache Klasse, die das Ergebnis der Serveroperation einkapselt, das dort auch intern für viele Funktionen verwendet wird.

class OpResult 
{ 
    ..... 
    bool succeeded(); 
    bool failed(); .... 
    ... data error/result message ... 
}; 

Als ich versuche, alle Funktionen klein und einfach zu haben, viele Blöcke wie folgt entstehen:

.... 
OpResult result = some_(mostly check)function(....); 
if (result.failed()) 
    return result; 
... 

Die Frage ist, ist es eine schlechte Praxis zu machen Makro wie folgt aussehen und es verwenden, überall?

#define RETURN_IF_FAILED(call) \ 
    {       \ 
    OpResult result = call; \ 
    if (result.failed())  \ 
     return result;   \ 
    } 

Ich verstehe, dass jemand es böse nennen kann, aber gibt es einen besseren Weg? Welchen anderen Weg, um Ergebnisse zu verarbeiten und viel Bloat-Code zu vermeiden, würden Sie vorschlagen?

Answer 1

Es ist ein Kompromiss. Sie handeln mit der Codegröße für die Verschleierung der Logik. Ich bevorzuge es, die Logik als sichtbar zu erhalten.

Ich mag Makros dieses Typs nicht, weil sie Intellisense (unter Windows) und das Debuggen der Programmlogik unterbrechen. Versuchen Sie einen Haltepunkt auf alle 10 return Anweisungen in Ihrer Funktion zu setzen - nicht die Prüfung, nur die return. Versuchen Sie, durch den Code zu gehen, der in dem Makro ist.

Das Schlimmste daran ist, dass es schwer ist, gegen die 30-Zeilen-Monstermakros zu argumentieren, die manche Programmierer gerne für gewöhnlich gesehene Mini-Aufgaben verwenden, weil sie "Dinge klären". Ich habe Code gesehen, bei dem verschiedene Exception-Typen auf diese Weise durch vier kaskadierende Makros behandelt wurden, was zu 4 Zeilen in der Quelldatei führte, wobei die Makros tatsächlich auf> 100 echte Zeilen expandierten. Reduzierst du jetzt Code Bloat? Nein. Es ist unmöglich, mit Makros leicht zu sagen. Ein weiteres allgemeines Argument gegen Makros, auch wenn es hier nicht offensichtlich anwendbar ist, ist die Fähigkeit, sie mit schwer entzifferbaren Ergebnissen zu verschachteln oder Argumente zu übergeben, die zu seltsamen, aber kompilierbaren Argumenten führen, z. die Verwendung von ++x in einem Makro, das das Argument zweimal verwendet. Ich weiß immer, wo ich mit dem Code stehe, und das kann ich über ein Makro nicht sagen.

EDIT: Ein Kommentar, den ich hinzufügen sollte, ist, dass, wenn Sie wirklich wiederholen diese Fehler überprüfen Logik immer und immer wieder, gibt es Refactoring Möglichkeiten im Code. Keine Garantie, aber ein besserer Weg zur Code-Blotreduzierung, wenn es zutrifft. Suchen Sie nach wiederholten Sequenzen von Aufrufen und kapseln Sie gemeinsame Sequenzen in ihrer eigenen Funktion, anstatt zu adressieren, wie jeder Aufruf isoliert behandelt wird.

Answer 2

Eigentlich bevorzuge ich etwas andere Lösung. Die Sache ist, dass das Ergebnis des inneren Anrufs nicht unbedingt ein gültiges Ergebnis eines äußeren Anrufs ist. Zum Beispiel kann der innere Fehler "Datei nicht gefunden" sein, aber der äußere "Konfiguration nicht verfügbar". Daher ist mein Vorschlag, das OpResult neu zu erstellen (potentiell das "innere" OpResult für besseres Debugging darin einzupacken). Dies alles geht in Richtung "InnerException" in .NET.

technisch, das Makro in meinem Fall sieht aus wie

#define RETURN_IF_FAILED(call, outerresult) \ 
    {           \ 
    OpResult innerresult = call;   \ 
    if (innerresult.failed())    \ 
    {          \ 
     outerresult.setInner(innerresult); \ 
     return outerresult;     \ 
    }          \ 
    } 

Diese Lösung jedoch einige Management-Speicher erfordert usw.

Einige Puristen argumentieren, dass ohne explizite Rückgabe die Lesbarkeit des Codes behindert wird. Meiner Meinung nach jedoch, RETURN ausdrücklich als Teil des Makronamens zu haben, ist genug, Verwirrung für jeden fähigen und aufmerksamen Entwickler zu verhindern.

---

Meine Meinung ist, dass eine solche Makros die Programmlogik nicht verschleiern, sondern im Gegenteil machen es sauberer. Mit einem solchen Makro deklarieren Sie Ihre Absicht klar und prägnant, während der andere Weg übertrieben ausführlich und damit fehleranfällig erscheint. Making die Betreuer im Verstand das gleiche Konstrukt OpResult r = call(); if (r.failed) return r ist verschwenden ihre Zeit.

Ein alternativer Ansatz ohne frühzeitige Rückgabe gilt für jede Codezeile das Muster CHECKEDCALL(r, call) mit #define CHECKEDCALL(r, call) do { if (r.succeeded) r = call; } while(false). Dies ist in meinen Augen viel viel schlimmer und auf jeden Fall fehleranfällig, da die Leute dazu neigen, beim Hinzufügen von mehr Code CHECKEDCALL() zu vergessen.

Mit einem populären müssen überprüft werden, kehrt zurück (oder alles) mit Makros scheint ein leichtes Zeichen für fehlende Sprachfunktion für mich zu sein.

Answer 3

Solange die Makrodefinition in einer Implementierungsdatei sitzt und nicht definiert ist, sobald sie nicht benötigt wird, wäre ich nicht entsetzt.

// something.cpp 

#define RETURN_IF_FAILED() /* ... */ 

void f1() { /* ... */ } 
void f2() { /* ... */ } 

#undef RETURN_IF_FAILED 

Allerdings würde ich dies nur verwenden, nachdem alle Nicht-Makro-Lösungen ausgeschlossen wurden.

Answer 4

Ich stimme zu Steve's POV.

Ich dachte zuerst, zumindest das Makro

#define RETURN_IF_FAILED(result) if(result.failed()) return result; 

zu reduzieren, aber dann zu mir, es kam dieser bereits ist ein Einzeiler, so gibt es wirklich wenig Nutzen im Makro.

---

Ich denke, im Grunde genommen müssen Sie einen Kompromiss zwischen Schreibfähigkeit und Lesbarkeit machen. Das Makro ist definitiv einfacher zu schreiben. Es ist jedoch eine offene Frage, ob es auch einfacher ist zu lesen. Letzteres ist ein ziemlich subjektives Urteil. Immer noch, objektive Makros verwendet Code verschleiern.

---

Letztlich ist das zugrunde liegende Problem ist, dass Sie keine Ausnahmen verwenden. Sie haben nicht gesagt, was die Gründe für diese Entscheidung sind, aber ich hoffe, dass sie die Probleme, die dadurch verursacht werden, wert sind.

Answer 5

Konnte mit C++ 0x lambdas getan werden.

template<typename F> inline OpResult if_failed(OpResult a, F f) { 
    if (a.failed()) 
     return a; 
    else 
     return f(); 
}; 

OpResult something() { 
    int mah_var = 0; 
    OpResult x = do_something(); 
    return if_failed(x, [&]() -> OpResult { 
     std::cout << mah_var; 
     return f; 
    }); 
}; 

Wenn Sie schlau und verzweifelt sind, könnten Sie die gleiche Art von Trick mit regulären Objekten arbeiten.

Answer 6

Meiner Meinung nach ist das Ausblenden einer return-Anweisung in einem Makro eine schlechte Idee. Die 'Code Obfucation' (Ich mag diesen Begriff ..!) Erreicht das höchstmögliche Niveau.Meine übliche Lösung für solche Probleme ist es, die Funktionsausführung an einem Ort zu sammeln und das Ergebnis in der folgenden Art und Weise steuern (vorausgesetzt, Sie haben 5 nullary Funktionen):

std::array<std::function<OpResult()>, 5> tFunctions = { 
f1, f2, f3, f4, f5 
}; 

auto tFirstFailed = std::find_if(tFunctions.begin(), tFunctions.end(), 
    [] (std::function<OpResult()>& pFunc) -> bool { 
     return pFunc().failed(); 
    }); 

if (tFirstFailed != tFunctions.end()) { 
// tFirstFailed is the first function which failed... 
} 

Answer 7

Gibt es Informationen in Folge, die tatsächlich nützlich ist, wenn Der Anruf schlägt fehl?

Wenn nicht, dann

static const error_result = something; 

if (call().failed()) return error_result; 

ausreichen würde.