Sauberer Code trotz goto ???



  • Danke für die Info's zu RAII. Das war genau das was ich gesucht habe. 👍



  • #define END_MY_FUNCTION delete[] m; \
      delete[] n; \
      return
    
    void DoSomething(unsigned int a) 
    { 
      // Reserviere Speicher, öffne Handles, .... 
      int m = new int[100]; 
      double n = new double[100]; 
      ... 
      // Mach was 
      if (a > 5) 
      { 
        if (!MyFunction(m, n, a)) 
        { 
           printf("Error 100\n"); 
           END_MY_FUNCTION;
        } 
        for (unsigned int i = 0; i < a; i++) 
        { 
          if (!MyFunction2(i)) 
            END_MY_FUNCTION;
          if (a*a > 100) 
          { 
             if (!MyFunction3(m, i)) 
             { 
               printf("Error 100\n"); 
               END_MY_FUNCTION;
             } 
             else if (a * a > 1000) 
               END_MY_FUNCTION;
          } 
        } 
        ... 
      } 
      END_MY_FUNCTION;
      ... 
    }
    


  • asdfasd schrieb:

    #define END_MY_FUNCTION delete[] m; \
      delete[] n; \
      return
    
    void DoSomething(unsigned int a) 
    { 
      // Reserviere Speicher, öffne Handles, .... 
      int m = new int[100]; 
      double n = new double[100]; 
      ... 
      // Mach was 
      if (a > 5) 
      { 
        if (!MyFunction(m, n, a)) 
        { 
           printf("Error 100\n"); 
           END_MY_FUNCTION;
        } 
        for (unsigned int i = 0; i < a; i++) 
        { 
          if (!MyFunction2(i)) 
            END_MY_FUNCTION;
          if (a*a > 100) 
          { 
             if (!MyFunction3(m, i)) 
             { 
               printf("Error 100\n"); 
               END_MY_FUNCTION;
             } 
             else if (a * a > 1000) 
               END_MY_FUNCTION;
          } 
        } 
        ... 
      } 
      END_MY_FUNCTION;
      ... 
    }
    

    Davon abgesehen das es eine scheiss Lösung, ist sie auch noch fehlerhaft.



  • Bitte ein Bit schrieb:

    Ich habe vier WINAPI-Handles offen (zwei Registry Handles, ein Mutex Handle und ein COM Port Handle), welche ich am Ende schliessen möchte, ...

    Für Registry- und Mutex-Handles gibt es fertige RAII-Lösungen:
    CRegKey: http://msdn.microsoft.com/en-us/library/xka57xy4(VS.80).aspx
    CMutex: http://msdn.microsoft.com/en-us/library/tt45160e(v=VS.71).aspx
    Falls du mit dem Com Port über die normalen File-Funktionen arbeitest (Gibt es überhaupt andere Möglichkeiten?) gibt es dafür auch CFile: http://msdn.microsoft.com/en-us/library/60fh2b6f(VS.80).aspx

    Die Klassen kümmern sich jeweils im Destruktor selbst um das Schließen der Handles.


  • Administrator

    rean schrieb:

    Bitte ein Bit schrieb:

    Ich habe vier WINAPI-Handles offen (zwei Registry Handles, ein Mutex Handle und ein COM Port Handle), welche ich am Ende schliessen möchte, ...

    Für Registry- und Mutex-Handles gibt es fertige RAII-Lösungen:
    CRegKey: http://msdn.microsoft.com/en-us/library/xka57xy4(VS.80).aspx
    CMutex: http://msdn.microsoft.com/en-us/library/tt45160e(v=VS.71).aspx
    Falls du mit dem Com Port über die normalen File-Funktionen arbeitest (Gibt es überhaupt andere Möglichkeiten?) gibt es dafür auch CFile: http://msdn.microsoft.com/en-us/library/60fh2b6f(VS.80).aspx

    Die Klassen kümmern sich jeweils im Destruktor selbst um das Schließen der Handles.

    Das sind aber ATL/MFC Klassen. Die bekommt man nicht gratis, dazu muss man sich eine Visual Studio Version Standard oder besser kaufen. Sowas kann man sich aber relativ leicht auch selber nachbauen, auch wenn es am Ende nur ein simpler Guard ohne Funktionen ist. Allerdings denke ich, dass es im Internet bereits fertige Klassen gäbe, wobei ich keine direkt kenne.

    Grüssli



  • @rean
    Cool. Ich kenne die Klassen gar nicht.

    @asdfasd
    Deine Lösung hat den massiven Nachteil dass das Debugging zur Compilezeit und Laufzeit schwer wird. Bau mal ein Fehler in dein Makro ein und versuche mal zu debuggen. :p

    Nichtsdestotrotz habe ich aber deine Lösung auch schon in fremden Projekten entdeckt.



  • Bitte ein Bit schrieb:

    Deine Lösung hat den massiven Nachteil dass das Debugging zur Compilezeit und Laufzeit schwer wird. Bau mal ein Fehler in dein Makro ein und versuche mal zu debuggen. :p

    Da sind schon zwei Fehler drin. Makros die mehrere Befehle einfügen aber nicht umklammert sind, sorgen schnell für Probleme, wie auch bei seinem Beispiel.



  • Janjan schrieb:

    Lösung = if + bool
    Sauberer Code trotz goto geht nicht.

    Ganz schlecht.



  • volkard schrieb:

    Janjan schrieb:

    Lösung = if + bool
    Sauberer Code trotz goto geht nicht.

    Ganz schlecht.

    Aber sowas von nicht schlecht du goto-Liebhaber!



  • Bitte ein Bit schrieb:

    Frage: Gibt es eine Möglichkeit das Ganze in einem sauberen Stil zu schreiben ? 😕

    Das *ist* guter Stil, soger allerbester Stil in C. Besser als lokal mit Exceptions herumzuwerfen oder künstliche Flags oder state machines oder deleteMitZeigerNullen oder ummantelnde Ressourcenhaltefunktionen. Bin mir sicher, mit genügend Zeit kann man Dutzende von Pseudolösungen finden, die aus dem Dilemma nicht wirklich herausführen. In C++ mit RAII haben wir genau für dieses Problem einen eleganten Ausweg und brauchen goto nicht mehr.



  • @Fellhuhn
    Ups, hast Recht. Ich hatte vergessen dass ein #define nichts anderes als ein Wortersetzungssystem ist.



  • In C++ gibt es das erwähnt RAII und zur Fehlerbehanldung Exceptions.

    void DoSomething(unsigned int a) 
    { 
      array<int, 100> m; 
      array<double, 100> n;
      // oder
      vector<int> m(100);
      vector<double> n(100);
      //... 
    
      if (a > 5) 
      { 
        try{
    
        MyFunction(m, n, a); // throws
        for (unsigned int i = 0; i < a; i++) 
        { 
          MyFunction2(i); // throws
          if (a*a > 100) 
          { 
             MyFunction3(m, i)) // throws
             else if (a * a > 1000) 
               throw logical_error("Error XYZ");
          } 
        } 
        catch(exception & e)
        {
         ShowMessage(e.what());
        }
      }
      // cleanup done in dtors
    }
    


  • brotbernd schrieb:

    try{
         //...
             else if (a * a > 1000) 
               throw logical_error("Error XYZ");
          } 
        } 
        catch(exception & e)
        {
         ShowMessage(e.what());
        }
    

    Exceptions zu benutzen um innerhalb der Funktion rumzuhüpfen ist nicht wirklich besser als goto. Exceptions sind dazu da um Fehler über Funktionsgrenzen hinweg weiterzugeben. Im besagten Fall kannst du an Stelle des throw gleich dein ShowMessage einbauen und mit return aus der Funktion verduften.

    volkard schrieb:

    Das *ist* guter Stil, soger allerbester Stil in C.

    In C ist das guter Stil, ja. Aber es geht ja um C++, und auch in der Beziehung ist C++ eben nicht mit C zu vergleichen, du sagst es ja selbst:

    In C++ mit RAII haben wir genau für dieses Problem einen eleganten Ausweg und brauchen goto nicht mehr.

    RAII ist die sauberere und wartbarere Alternative zu goto in C++, und in Punkto Exceptionsicherheit ist goto völlig daneben und deswegen in C++ ganz anders als in C schlechter Stil.
    Und bevor jetzt jemand sagt "aber wenn ich garkeine Exceptions hab ist es auch kein schlechter Stil.", dem sei erwidert, dass Exceptions ein allgegenwärtiges Sprachfeature von C++ sind. Wenn man selbst sich plötzlich entscheidet, Exceptions einzusetzen oder eine Bibliothek einzusetzen, die Exceptions verwendet, dann muss man plötzlich sein goto-Gefrickel doch umstellen. => goto ist auch in Abwesenheit von Exceptions schlechter Stil weil es die Erweiterbarkeit unnötig einschränkt.

    Nachtrag: wie alle Diskussionen die das Wort "Stil" enthalten ist auch diese zu mindestens 50% subjektiver Natur 😉



  • pumuckl schrieb:

    Exceptions zu benutzen um innerhalb der Funktion rumzuhüpfen ist nicht wirklich besser als goto. Exceptions sind dazu da um Fehler über Funktionsgrenzen hinweg weiterzugeben.

    Das Programm bestand aber nur aus einer Funktion.

    pumuckl schrieb:

    Im besagten Fall kannst du an Stelle des throw gleich dein ShowMessage einbauen und mit return aus der Funktion verduften.

    Welchen Vorteil habe ich davon, diesen Fehler an anderer Stelle zu behandeln als die anderen. Wenn ich mich später entscheide z.B. einen Fehlerbericht zu schreiben, führe ich im catch Block entsprechende Funktionalität hinzu. Fehler die sich irgendwo anders verabschieden, schummeln sich dann daran vorbei. Ich kann das natürlich auch in eine HandleError Funktion oder was weiß ich kapseln, aber einen wirklichen Nachteil der 'lokalen' Ausnahme seh ich jetzt nicht (ich mach das selber aber auch fast nie, weil meine Programme meistens umfangreicher als eine Funktion sind). Bezüglich eventueller Probleme lass ich mich aber gerne belehren. 😉



  • pumuckl schrieb:

    volkard schrieb:

    Das *ist* guter Stil, soger allerbester Stil in C.

    Aber

    Ups. Die Satzzeichen waren total falsch.
    Wollte sagen, daß es in C bester Stil ist. In C++ gar nicht.



  • Eine noch ungenannte Lösung (zumindest sehe ich sie auf Anhieb nicht) ist auch einen Teil der Funktion in eine zweite auszulagern, und diese bei Bedarf mit return zu beenden (und dieser als Parameter die nötigen Daten mitzugeben).

    Spricht goto durch return in einer Funktion auslagern.



  • asc schrieb:

    Eine noch ungenannte Lösung (zumindest sehe ich sie auf Anhieb nicht) ist auch einen Teil der Funktion in eine zweite auszulagern, und diese bei Bedarf mit return zu beenden (und dieser als Parameter die nötigen Daten mitzugeben).

    Spricht goto durch return in einer Funktion auslagern.

    Hab ich als "ummantelnde Ressourcenhaltefunktionen" erwähnt und als nicht so gut wie RAII abgetan.



  • volkard schrieb:

    asc schrieb:

    Eine noch ungenannte Lösung (zumindest sehe ich sie auf Anhieb nicht) ist auch einen Teil der Funktion in eine zweite auszulagern, und diese bei Bedarf mit return zu beenden (und dieser als Parameter die nötigen Daten mitzugeben).

    Spricht goto durch return in einer Funktion auslagern.

    Hab ich als "ummantelnde Ressourcenhaltefunktionen" erwähnt und als nicht so gut wie RAII abgetan.

    Für den Punkt, dass es hier um die Ressourcen geht stimme ich dir zu. Allerdings kann man bei Tiefen wie hier schon mal dran denken das in Funktionen auszulagern.

    if{for{if{if{..}}}}
    

    ist imo schon recht tief.

    Noch nicht schreklich, aber man sollte sich da schon mal überlegen, ob es besser geht.



  • Dravere schrieb:

    Das sind aber ATL/MFC Klassen. Die bekommt man nicht gratis, dazu muss man sich eine Visual Studio Version Standard oder besser kaufen. Sowas kann man sich aber relativ leicht auch selber nachbauen, auch wenn es am Ende nur ein simpler Guard ohne Funktionen ist. Allerdings denke ich, dass es im Internet bereits fertige Klassen gäbe, wobei ich keine direkt kenne.

    Grüssli

    Dann bastelt man sich eben nen schönen kleinen RAII-Wrapper mit ein wenig spicken bei boost::any:

    class RAIIClose
    {
    	class placeholder
    	{ public: virtual ~placeholder(){} };
    
    	template< typename ValTy, typename Func >
    	class holder : public placeholder
    	{
    		ValTy &held;
    		Func func_;
    
    	public:
    
    		holder( ValTy & value, Func func )
    			: held( value ), func_( func )
    		{}
    
    		~holder()
    		{ func_( held ); }
    	};
    
    	placeholder * content;
    
    public:
    
    	template< typename ValTy, typename Func >
    	RAIIClose( ValTy & value, Func func )
    		: content( new holder< ValTy, Func >( value, func ) )
    	{}
    
    	~RAIIClose()
    	{ delete content; }
    };
    
    //Usage:
    {
    	HKEY hkey;
    	HANDLE file;
    
    	RAIIClose x1( hkey, RegCloseKey );		
    	RAIIClose x2( file, CloseHandle );
    
    	//...
    }//RAII räumt für uns auf
    

    Bei häufiger Verwendung würd ich aber, wie Dravere schon gesagt hat, selbst was schreiben oder sowas verwenden:

    #define HANDLE_WRAPPER( HW_HANDLE, HW_FUNC, HW_NAME )	\
    class HW_NAME											\
    {														\
    	HW_HANDLE handle_;									\
    public:													\
    	operator HW_HANDLE&(){ return handle_; }			\
    														\
    	void operator=( const HW_HANDLE& handle )			\
    	{ handle_ = handle; }								\
    														\
    	~HW_NAME()											\
    	{ HW_FUNC( handle_ ); }								\
    };;
    
    HANDLE_WRAPPER( HKEY, RegCloseKey, wRegKey );
    HANDLE_WRAPPER( HANDLE, CloseHandle, wMutex );
    
    //Usage
    
    {
    	wRegKey hkey = HKEY_USERS;
    	wMutex m = CreateMutex( NULL, TRUE, NULL );
    
    	//...
    
    }//RAII räumt für uns auf
    

    Vielleicht kann man das zweite noch irgendwie in ein Template packen, bin grad nur zu müde zum rumprobieren.



  • volkard schrieb:

    pumuckl schrieb:

    volkard schrieb:

    Das *ist* guter Stil, soger allerbester Stil in C.

    Aber

    Ups. Die Satzzeichen waren total falsch.
    Wollte sagen, daß es in C bester Stil ist. In C++ gar nicht.

    volkard schrieb:

    SATZZEICHEN KÖNNEN LEBEN RETTEN

    Sauberer Code trotz goto ???

    wenn einer die frage stellt, dann ist sein code nicht sauber.


Anmelden zum Antworten