Ist das guter Stil?



  • Wenn du Angst vor Leerräumen hast, musst du dir wohl ein anderes Hobby suchen :p



  • Nein, Unterstriche sind hässlicher Schrott aus der Programmierer-Steinzeit. Für Trenner zwischen Wortteilen gibt es verschiedene Ansichten dazu, aber am Anfang oder Ende aus irgendwelchen fadenscheinigen Gründen Unterstriche zu machen, ist Schrott.

    Und ohne Leerzeichen kann man deinen code kaum lesen, weil man einfach nicht weiß, was zusammengehört.



  • Optimizer: Wollt ich auch grad schreiben, also:

    Full ACK.



  • @Optimizer also den code mit "this->" unlesbar machen, wenn die Kreativität beim funktionsparameter versagt? Was machen, wenn der Typname eigentlich der einzig sinnvolle variablenname ist?(beispiel: Tokenizer tokenizer;)



  • Also ich finde this-> deutlich schöner und lesbarer, als diese komischen m_member und member_ und Kreationen. (Und _ am Anfang ist doch AFAIK für den Compilerhersteller reserviert, oder?)



  • Otze also du solltest definitiv stärker mit Leerzeichen/-zeilen arbeiten und du solltest
    die öffnende geschweifte Klammer in eine eigene Zeile setzen, ich habe es lange Zeit
    auch so wie du es zZ hast gemacht, aber es ist deutlich einfacher zu lesen, wenn sie
    eine eigene Zeile bekommt.

    Führende Unterstriche usw. kannste immer weglassen, hol dir lieber eine IDE die Member-,Globale-,Lokale-Variablen unterschiedlich formatiert/einfärbt.

    Mit this-> arbeiten ist nix schlimmes, wenn die Parameter die gleichen Namen haben.

    Und das mit dem Klassenamen und Variablenamen ist bei mir oft auch so, aber man erkennt dank Highlighting ja eh immer was was ist.



  • Schreib einfach m_ vor Member, oder its (obwohl das sehr unbeliebt ist).

    MfG SideWinder



  • Optimizer schrieb:

    deine Unterstriche kann man gar nicht schön finden (davon abgesehen, dass solche Namen nur halblegal sind).

    Doch, kann man. Und die sind auch nicht nur halblegal, sondern völlig legal.



  • DrGreenthumb schrieb:

    Optimizer schrieb:

    deine Unterstriche kann man gar nicht schön finden (davon abgesehen, dass solche Namen nur halblegal sind).

    Doch, kann man. Und die sind auch nicht nur halblegal, sondern völlig legal.

    Das war doch so, dass der Buchstabe nach dem führenden Unterstrich nicht groß sein darf, oder? Hab grad keinen Standard zur Hand... Trotzdem finde ich es hässlich!



  • Ich kann mich der Vorderung nach mehr Leerstellen nur anschließen, besonders Leerzeilen, würde ich nach jeder Funktion- oder Klassendefinition machen. Wobei ich bei Operatoren aber auch keine Leerstellen mache. Die Klammern finde ich so übrigens gut.

    Die Memberbezeichnung ist so eine Sache, die jeder gerne anders macht. Ich persönlich mach immer _m als postfix.

    @DrGreenthumb
    doch, dass kann schon illegal sein, wenn es sich (a) um einen globalen Bezeichner handelt (ist bei Membern ja ausgeschlossen) oder (b) wenn ein Großbuchstabe folgt.

    http://www.c-plusplus.net/forum/viewtopic.php?t=39461



  • Das mit den Leerzeilen etc. wurde ja bereits erwähnt. Zu der Diskussion wegen
    der Membermarkierung find ich den Unterstrich am Ende recht übersichtlich.

    Aber zu was andrem:

    std::string convert(const std::string& token,MTL::Identity<std::string>)const{
        std::string string(token);
        // ...
    

    Warum übergibst du hier den string per Referenz ? Das einzige was du mit dem
    machst is ihn kopieren um die Kopie verändern zu dürfen. Warum also nicht gleich
    Call by Value ?

    if(*String.end()-1=='\"'){
        string.erase(string.end()-1);
    }
    

    (Ich nehm mal an, dass mit String eigentlich string gemeint ist)
    Der operator* hat eine höhere Priorität als operator-, und wie du sicher
    weißt is .end() dereferenzieren böse !

    Was noch fehlt sind Kommentare, die den Sinn von etwas beschreiben.
    Zum Beispiel is folgender Code trotzdem verdammt unklar:

    int size=string.size();
    return std::string((char*)&size,sizeof(int))+=string;
    


  • std::string convert(const std::string& token,MTL::Identity<std::string>)const{
        std::string string(token);
        // ...
    

    dazu gabs letztens ne diskussion, bei der herauskam, dass der compiler hierbei schnelleren code erzeugt/erzeugen kann,als bei call by value. Ausserdem ist es einheitlicher und übersichtlicher.

    if(*string.end()-1=='\"'){
        string.erase(string.end()-1);
    }
    

    das compiliert der gcc 1. ohne probleme und 2. absolut richtig,auch wenn ich atm keine tabelle dazu zur Hand hab, der gcc sollte es wohl richtig implementiert haben.
    //edit ähm seit wann hat std::string einen op- der int annimmt? das war mir bisher neu, und hat mich davon abgehalten, diesen Fehler zu finden, danke tankian

    int size=string.size();
    return std::string((char*)&size,sizeof(int))+=string;
    

    k kommentare werden hinzugefügt 🙂
    //edit ich hab das jetzt nochmal überarbeitet, soll ich den ersten post updaten, oder lieber einen separaten post dafür anlegen?



  • kingruedi schrieb:

    @DrGreenthumb
    doch, dass kann schon illegal sein, wenn es sich (a) um einen globalen Bezeichner handelt (ist bei Membern ja ausgeschlossen) oder (b) wenn ein Großbuchstabe folgt.

    ja, ich weiß. Aber da es ja hier um Member geht, war die Aussage halt falsch.

    Tankian schrieb:

    std::string convert(const std::string& token,MTL::Identity<std::string>)const{
        std::string string(token);
        // ...
    

    Warum übergibst du hier den string per Referenz ? Das einzige was du mit dem
    machst is ihn kopieren um die Kopie verändern zu dürfen. Warum also nicht gleich
    Call by Value ?

    hm, ich mach das auch immer so. Finde ich irgendwie klarer.



  • achja, was ich noch vergessen habe.

    Ich finde, dass man die Klammern bei if weglassen sollte, wenn man nur eine Anweisung in der nächsten Zeile hat.

    int size=string.size();
    

    und hier ist int falsch, da musst du std::size_t oder std::string::size_type benutzen.

    Und ich versteh nicht, warum du von der ConvertPolicy erbst.



  • Und ich versteh nicht, warum du von der ConvertPolicy erbst.

    ganz einfach, weil ich nicht weis, wie die entsprechende implementation aussehen kann, vielleicht soll es in irgendeiner implementation möglich sein,einen Filter zu setzen, der irgendwas mit den Werten macht, und für sowas kann es ja eine neue public methode setFilter oder ähnliches geben. 🙂



  • Sieht aus als wäre dies nur eine Auschnitt aus nem grösseren Projekt. 🙂
    In diesem Fall würde ich ne einheitliche Kommentierung mit Unterstützung
    eines Dokumentationstool z.B. DoxyGen einfügen. Das kann anderen und auch
    dir selbst mal irgendwann das Leben sehr erleichtern.

    mfg JJ



  • DOKUMENTATION FEHLT! (SEHR WICHTIG)



  • Auch wenn es bestimmt schon erwähnt wurde:

    Leute bitte, coded wie ihr wollt aber spendiert jeder "{" eine eigene Zeile! Das ist eifnach nur grausig das hinter den Funktionskopf zu schreiben. Das blickt keiner mehr durch *anfleh*



  • #ifndef TOKEN_CONVERTER_HPP
    #define TOKEN_CONVERTER_HPP 
    #include <string> 
    #include <sstream> 
    #include <MTL/Identity.hpp>
    
      class StringConverter  { // Klammern mit Leerzeichen, nicht direkt an letztes Zeichen
         protected: 
             template<class T> 
             T convert(const std::string& token, MTL::Identity<T>)const
                { 
                 std::stringstream stream; 
                 stream << token; 
                 T output; 
                 stream >> output; 
                 return output; 
                } 
    
         std::string convert(const std::string& token,MTL::Identity<std::string>) const
         { 
           std::string string(token); 
    
            if(*string.begin() == '\"') 
              { 
               string.erase(string.begin()); 
              } 
    
            if(*string.end()-1 == '\"')
              { 
               string.erase(string.end() -1); 
              } 
    
          int size = string.size(); 
    
          return std::string((char*)&size,sizeof(int)) += string;
    
         }
    /* meine implementation der STL unterstützt kein insert(size_t,const 
       charT*,size_t),deshalb diese Unschöne zeile */ 
    
                          }; //class ENDE
    
    /*
    ##########################################################################################
    ## Mithilfe dieser Klasse wird ein tokenstream konvertiert,dessen typen bereits bekannt ##
    ## sind, Ist dies nicht der Fall, so arbeitet diese Klasse sehr gut mit der Klasse      ##
    ## Categorizer in der datei Categorizer.hpp zusammen. Das erste template gibt den Typ   ##
    ## der Iteratoren an, die den Tokenstream "darstellen, das zweite Template eine Policy  ##
    ## für die Konvertierung der Token.                                                     ##
    ##########################################################################################
    */
    
    template<class Iterator,class ConvertPolicy = StringConverter> 
      class TokenConverter:public ConvertPolicy  { 
          public: 
             typedef Iterator iterator; 
    
          private: 
             const iterator _begin; 
             mutable iterator _pos; 
             const iterator _end; 
    
             template<class T> 
             T convertToken()const 
             { 
                 return ConvertPolicy::convert(*_pos,MTL::Identity<T>()); 
             } 
    
             public: // Diese Einrückung war vorher falsch !!
             TokenConverter(const iterator& begin,iterator& pos,const iterator& end):_begin(begin), _pos(pos), _end(end) {} //Leer nach Komma 
             const iterator getPos()const
             { 
                 return _pos; 
             } 
    
             void setPos(iterator pos)const
             { 
                 _pos = pos; // unterstrich ist böse
             } 
    
             template<class T> 
               T getNext()const
               { 
                 assert(_pos != _end); // Leerzeichen assert(_pos!=_end);
                 ++_pos; 
                 return convertToken<T>(); 
               } 
    
             template<class T> 
               T getPrevious()const
               { 
                 assert(_pos != _begin); 
                 --_pos; 
                 return convertToken<T>(); 
               } 
    
                                               }; // class ENDE
    
    #endif
    
    /*
    Nach Komma Leerzeichen verwenden
    Unterstriche = nicht gut
    Zwischen den Operatoren -> Leerzeichen -> variable=sowieso -> variable = sowieso ist lesbarer
    Einrückungen so wählen das man erkennt wozu es gehört
    KlassenEnden kennzeichnen macht auch Sinn, auch wenn man "kleine" Klassen hat.
    if Konstrukte zu Klammern finde ich gut, bei einem Nachtrag vergisst man sie nicht.
    Dokumentation = warum und nicht was. Warum tu ich es nicht wie.
    Wer nicht weiss "wie" der muß es lernen. Warum ich einen Ausdruck so definiere macht da schon eher Sinn.
    */
    


  • das ist aber mal echt krass unübersichtlich. die tabs solltenw enigstens eingehalten werden. und wieso das eine public bei dir anders eingerückt wurde frag ich mich immer noch.

    najut, nun meine neue version:

    #ifndef TOKEN_CONVERTER_HPP
    #define TOKEN_CONVERTER_HPP
    #include <string>
    #include <sstream>
    #include <MTL/Identity.hpp>
    
    class StringConverter{
    	protected:
    		template<class T>
    		T convert(const std::string& token,MTL::Identity<T>)const{
    			std::stringstream stream;
    			T output;
    
    			stream<<token;
    			stream>>output;
    
    			return output;
    		}
    
    		//ein string wird anders konfertiert: als ausgabe kommt zuerst seine länge,
    		//gefolgt von seinem Inhalt ohne führende und abschließende \"
    		std::string convert(const std::string& token,MTL::Identity<std::string>)const{
    			std::string string(token);
    
    			if(*string.begin()=='\"'){
    				string.erase(string.begin());
    			}
    			if(*(string.end()-1)=='\"'){
    					string.erase(string.end()-1);
    			}
    
    			std::size_t size=string.size();
    			return std::string((char*)&size,sizeof(int))+=string ;
    		}
    };
    
    ////////////////////////////////////////////////////////////////////////////////////////
    //Mithilfe dieser Klasse wird ein tokenstream konvertiert,dessen typen bereits bekannt//
    //sind, Ist dies nicht der Fall, so arbeitet diese Klasse sehr gut mit der Klasse     //
    //Categorizer in der datei Categorizer.hpp zusammen. Das erste template gibt den Typ  //
    //der Iteratoren an, die den Tokenstream "darstellen, das zweite Template eine Policy //
    //für die Konvertierung der Token.                                                    //
    ////////////////////////////////////////////////////////////////////////////////////////
    
    template<class Iterator,class ConvertPolicy=StringConverter>
    class TokenConverter:public ConvertPolicy{
    	public:
    		typedef Iterator iterator;
    
    	private:
    		const iterator begin;
    		mutable iterator pos;
    		const iterator end;
    
    		template<class T>
    		T convertToken()const{
    			return ConvertPolicy::convert(*pos,MTL::Identity<T>());
    		}
    
    	public:
    		TokenConverter(const iterator& begin,iterator& pos,const iterator& end):begin(begin),pos(pos),end(end){}
    
    		const iterator getPos()const{
    			return pos;
    		}
    		void setPos(iterator pos)const{
    			this->pos=pos;
    		}
    
    		template<class T>
    		T getNext()const{
    			assert(!(pos==end));
    			++pos;
    			return convertToken<T>();
    		}
    		template<class T>
    		T getPrevious()const{
    			assert(!(pos==begin));
    			--pos;
    			return convertToken<T>();
    		}
    };
    
    #endif
    

Anmelden zum Antworten