Ist das guter Stil?

[DrGreenthumb]

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.

[rüdiger]

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.

[otze]

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.

[John Doe]

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

[FULL ACK]

DOKUMENTATION FEHLT! (SEHR WICHTIG)

[uahhhhh]

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*

[U-U-U]

#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.
*/

[otze]

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

[DrGreenthumb]

U-U-U schrieb:

if(*string.begin() == '\"')
          {
           string.erase(string.begin());
          }

Diese Einrückung is ja richtig Banane. Da war die von otze besser...

Und Klassenenden kennzeichnen? Nee, das macht schon die }-Klammer. Ausserdem ist danach die Datei ohnehin meistens zu Ende.

[nman]

otze: Ich finde Deinen Code ziemlich lesbar.

DrGreenthumb schrieb:

U-U-U schrieb:

if(*string.begin() == '\"')
          {
           string.erase(string.begin());
          }

Diese Einrückung is ja richtig Banane. Da war die von otze besser...

Naja, ich mag diese Einrückungsart sehr gerne; nur hätte ich in so einem Fall ganz klar die geschwungenen Klammern ganz weggelassen.

edit: Oder meintest Du etwa das Einrücken der geschwungenen Klammern? Das finde ich nämlich eher bescheuert.

[Stil-Profi]

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

Das ist schlechter Stil.

[DrGreenthumb]

otze schrieb:

assert(!(pos==end));

pos != end war irgendwie logischer.

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

Das würde ich so schreiben:

pop_front(string, '\"');
pop_back(string, '\"');

[DrGreenthumb]

Die überflüssigen Klammern bei einzeiligen if-Blöcken sind überflüssig, also "schlechter Stil". Lässt sich ohne einfach besser lesen.

Wer meint die Klammern zu vergessen, wenn er neue Zeilen hinzufügt, soll sich 'nen Editor mit automatischer Einrückung besorgen.

[otze]

DrGreenthumb schrieb:

otze schrieb:

assert(!(pos==end));

pos != end war irgendwie logischer.

so sieht man aber sofort, wann der assert ausgelöst wird,vorher musste ich immer umdenken,so seh ichs auf den ersten Blick.

pop_front(string, '\"');
pop_back(string, '\"');

kenn ich nicht :D,selbstgeschrieben?

[DrGreenthumb]

otze schrieb:

DrGreenthumb schrieb:

otze schrieb:

assert(!(pos==end));

pos != end war irgendwie logischer.

so sieht man aber sofort, wann der assert ausgelöst wird,vorher musste ich immer umdenken,so seh ichs auf den ersten Blick.

Ein assert ist doch Dokumentation. Die sagt hier klar und deutlich, pos muss ungleich end sein.

pop_front(string, '\"');
pop_back(string, '\"');

kenn ich nicht :D,selbstgeschrieben?

seit ewigkeiten in meiner str.h
Solche Kleinigkeiten erleichtern das lesen und schreiben ungemein.

[otze]

Ein assert ist doch Dokumentation. Die sagt hier klar und deutlich, pos muss ungleich end sein.

ein typischer Fall von: aus unterschiedlichen Blickwinkeln sehen
ich frage danach, wann der assert eintritt, und du, wann er nicht eintritt

[Gregor]

@otze: Ich habe den Thread jetzt zwar nicht wirklich gelesen, aber ich sag einfach mal meine Meinung zu Stilfragen:

Wenn man einen guten Stil hat, dann weiß man das selbst. Guter Stil heißt, dass man sich bei allen Entscheidungen über die vorhandenen Entscheidungsmöglichkeiten mit ihren Vor- und Nachteilen im Klaren ist und, dass man eine entsprechend große Wissens- und Erfahrungsgrundlage hat, anhand derer man bewußt eine bestimmte Variante auswählt.

Wenn du also bei deinem Code überall sagen kannst, warum du etwas wie gemacht hast und deine Entscheidungen auch gegenüber anderen rechtfertigen kannst, dann hast du guten Stil.

Diese Klammer-Fragen, die hier am Schluss mal wieder genannt wurden, haben nichts mit Stil zu tun. Wenn einem das nicht paßt, dann läßt man halt ein Tool drüberlaufen, das den Code nach den eigenen Vorstellungen formatiert.

[Ben04]

Man hat einen guten Stil wenn man seinen Code auch noch nach einem Jahr gelesen kriegt (meine Definition).

ein typischer Fall von: aus unterschiedlichen Blickwinkeln sehen
ich frage danach, wann der assert eintritt, und du, wann er nicht eintritt

Also ich lese:

assert(!(a==b))

als

Stelle sicher, dass nicht a gleich b ist.

und

assert(a!=b)

als

Stelle sicher, dass a ungleich b ist.

Wobei ich letzters wesentlich leichter verständlich finde. Aber sei es drum wenn du es gelesen kriegst ist es gut.