2017-03-30 9 views
0

Die Konstruktorfunktion für die Klassenroute enthielt ursprünglich den folgenden Code, der überprüft, ob ein Element ("gpx", "rte" usw.) in einer Datei vorhanden ist. Was so läuft wie es soll.Refactoring-Code in C++

if (! elementExists(source,"gpx")) 
    { 
     oss << endl << "no gpx tag"; 
     constructorReport = oss.str(); 
     constructorSucceeded = false; 
     return; 
    } 
    if (! elementExists(source,"rte")) 
    { 
     oss << endl << "no rte tag"; 
     constructorReport = oss.str(); 
     constructorSucceeded = false; 
     return; 
    } 

Ich habe versucht, eine Funktion einzuführen, diese if-Anweisungen zu ersetzen. Das Programm baut sich gut aus.

void Route::constCheck(string source, string type) 
{ 

    if (! XML_Parser::elementExists(source, type)) 
    { 
     std::ostringstream oss; 
     oss << std::endl << "no" << type <<" tag"; 
     constructorReport = oss.str(); 
     constructorSucceeded = false; 
     return; 
    } 
} 

Ich habe die GPX-Datei geändert, die sie überprüft, um einen Fehler zu produzieren, aber mit meiner zusätzlichen Funktion geht es weiter, als gäbe es keinen Fehler ist.

Jede Hilfe ist willkommen und bitte lassen Sie mich wissen, wenn Sie weitere Informationen benötigen. Ich habe versucht, das Code-Licht nach Richtlinien zu halten.

+2

Was denken Sie 'return' tut? –

+0

Rückgabe für Void-Funktionen sind nicht notwendig :) Aber das Problem liegt hier woanders. – 0xDEFACED

+0

Können Sie uns zeigen, wie und wo Sie Ihre neue Funktion ausführen? – 0xDEFACED

Antwort

0

In Ihrem ursprünglichen Code kehren Sie von der Funktion zurück, wenn einer der Tests zum ersten Mal fehlschlägt, Sie fahren nicht fort, die anderen Tests zu versuchen.

Jetzt, da Sie den Test in eine Funktion verschoben haben, kann der Aufrufer nicht mehr wissen, ob der Test fehlschlägt. Er führt also alle aus und kehrt nie von seiner Funktion zurück, wenn eine davon fehlschlägt. Sie benötigen diese Funktion, um einen booleschen Wert zurückzugeben, der angibt, ob ein Fehler aufgetreten ist.

bool Route::constCheck(string source, string type) 
{ 

    if (! XML_Parser::elementExists(source, type)) 
    { 
     std::ostringstream oss; 
     oss << std::endl << "no" << type <<" tag"; 
     constructorReport = oss.str(); 
     constructorSucceeded = false; 
     return false; 
    } 
    return true; 
} 

Dann würde der Ersatz für Ihre Original-Code wie folgt aussehen:

if (!constCheck(source, "gpx")) { 
    return; 
} 
if (!constCheck(source, "rte")) { 
    return; 
} 
+0

Ich verstehe, wo ich falsch gelaufen bin. Der Code funktionierte perfekt Ich musste nur die weglassen! wenn man es anruft. Danke für die Hilfe!. – JohnDoe