Suche SEHR gutes Beispiel für guten Code-Style
-
Hi,
es wird ja oft geredet über GUTEN code-style, nun da liegt die Frage offen:
Wie sieht guter Code-Style eigentlich aus?Ist es sowas:
namespace my_std { // Gibt kleinste Zahl von 'a' und 'b' zurück template <class T> inline const T min (const T& a, const T& b) { return (a < b ? a : b); } // Testet ob 'value' in einem Toleranzbereich von 'min' und 'max' ist. template <class T> inline const bool tolerance (const T& value, const T& min, const T& max) { return ((value >= min) && (value <= max)); } }sowas:
void precalcRGBcomponents (RGB_INFO *rgbInfo) { // Kopiere Parameter zu Global g_redPos = rgbInfo->posRed; g_greenPos = rgbInfo->posGreen; g_transColor = rgbInfo->transColor; for (int i=0; i<NUM_COLORS; ++i) { g_red [i] = static_cast<unsigned char>((i & rgbInfo->maskRed) >> rgbInfo->posRed); g_green [i] = static_cast<unsigned char>((i & rgbInfo->maskGreen) >> rgbInfo->posGreen); g_blue [i] = static_cast<unsigned char>((i & rgbInfo->maskBlue)); } }oder was weis ich was?
Wie sieht bei eucht guter Codingstyle in C++ aus? Zeigt mal ein paar Source-Ausschnitte
-
ich selber hab mir recht viel einrückung angewöhnt:
//======================================================================================= // // class CSprite // //======================================================================================= class CSprite { public: // ==== Konstruktor CSprite(); // ==== Destruktor virtual ~CSprite(); // ------------------- Main Methods void Create (LPDIRECT3DDEVICE9 lpDevice, CResourceManager* ResManager, int frmWidth, int frmHeight, int numberFramesX, int numberFramesY, int frmDelay); void AddTexture (LPDIRECT3DTEXTURE9 lpTexture); void Draw (); void Draw (int alphawert); void Draw (int destX, int destY); void DrawAnim (int destX, int destY, int FrameDelay); void Move (); void Move (int newX, int newY); void Move (int direction); BOOL TestCollision (vector <CSprite*> vecSprite); // ------------------- Set inline void SetWorldX (int newX) { m_vPosition.x = newX; } inline void SetWorldY (int newY) { m_vPosition.y = newY; } inline void SetPosition (float x,float y) { m_vPosition.x = x; m_vPosition.y = y; } inline void SetAlpha (int a) { alpha = a; } inline void SetAlphaEnabled (BOOL al) { alphaEnable = al; } inline void SetColorkey (int r, int g, int b) { R = r; G = g; B = b; } inline void SetAnim (BOOL anim) { animation = anim; } inline void SetRotation (float rotation) { m_Rotation = rotation; } inline void SetScale (float scale) { m_vScale.x = scale; m_vScale.y = scale; } inline void SetStep (float StepX,float StepY) { m_StepX = StepX; m_StepY = StepY; } inline void SetSpeed (float Speed) { m_CurSpeed = Speed; } inline void SetType (int type) { m_Type = type; } inline void SetID (int ID) { spriteID = ID; } inline void SetTexture (LPDIRECT3DTEXTURE9 newTexture) { m_AnimationVec[0] = newTexture; } // ------------------- GET inline int GetX () { return m_vPosition.x; } inline int GetY () { return m_vPosition.y; } inline int GetWorldX () { return worldX; } inline int GetWorldY () { return worldY; } inline int GetType () { return m_Type; } inline int GetHeight () { return m_Height; } inline int GetWidth () { return m_Width; } inline float GetRotation () { return m_Rotation; } inline D3DXVECTOR2 GetCenter () { return m_vRotationCenter; } inline D3DXVECTOR2 GetPosition () { return m_vPosition; } // ------------------- Other methods void Scale (float dScale); void Rotate (float dRotation); void Rotate (int Direction); void Bounce (); void Animate (); BOOL PlayedAnimation (); protected: LPDIRECT3DDEVICE9 m_lpDevice; LPD3DXSPRITE m_lpSprite; LPDIRECT3DSURFACE9 m_lpSpriteSurface; D3DXVECTOR2 m_vPosition; D3DXVECTOR2 m_vScale; D3DXVECTOR2 m_vRotationCenter; float m_Rotation; float m_RotationStep; float m_CurSpeed; float m_MaxSpeed; float m_Acceleration; float m_AnimationSpeed; float m_StepX; float m_StepY; int m_Width; int m_Height; float m_CurrentImage; CResourceManager* m_ResManager; vector<LPDIRECT3DTEXTURE9> m_AnimationVec; int m_Type; BOOL m_PlayedAnimation; BOOL animation; int delay; int CurrentFrameX; int CurrentFrameY; int FramesX,FramesY; int FrameWidth; int FrameHeight; int FrameDelay; int worldX; int worldY; int R; int G; int B; int spriteID; int alpha; string path; BOOL alphaEnable; };Das sieht hier noch etwas unordentlich aus, das liegt aber am Forum (:
stellt euch die sachen einfach ein wenig mehr geordnet untereinander vor.
-
Gut lesbar für Fremde und auch nach längerer Zeit für den Entwickler noch verständlich.
-
mit den einrückungen würd ichs nicht übertreiben
// funktionen: typ name(param, param); void myFunc(void param1, void param2) { // einmal einrücken (4 leer), istgleich untereinander int retval = 1; int ab1 = 2; int arr[][] = { {1, 2, 3}, {2, 3, 4} }; Class A; retval = funktionsAufruf(param, param); }wenn so übel eingerückt wird, wie 2 vor mir
void name (param param)kanns ja kein mensch mehr lesen
edit: ach ja: und funktionsparameter würd ich nicht untereinander schreiben, find ich unübersichtlich:
statt void vieleParams( int param1, char param2, ... ) { //... } lieber void vieleParams(int param1, char param2, ... char param90, int param20, ...) { }täuscht mich das, oder hat eh nur winapi solch endlose parameterlisten?
-
@Zyrian
der Code ist doch total hässlich, allein schon, dass du ein "C" als Klassen Prefix wählst, disqualifiziert ihn von jeglicher Teilnahme an einem C++-Code-Style Wettbewerb. Dann sind deine Member Variablen nicht geschützt (siehe Open-Closed-Prinzip), explizit inline bei Membern brauchst du auch nicht und das einrücken finde ich macht die Sache noch viel unleserlicher.Dann benutzt du auch noch BOOL anstelle von bool und um const-correctness kümmerst du dich auch nicht. Und diese hässlichen typedef LPDINGENSKIRCHEN sorgen auch für keine Schönheit an deinem Code.
Außerdem benutzt du für zB. größen Angaben einfach int, obwohl ein unsigned Typ viel logischer wär.
Dein dtor ist virtuell, aber keine andere Funktion, da sehe ich keinen Sinn.
Der erste Kommentar ist auch nicht hilfreich und nur redundant, da sollte lieber stehen, was die Klasse macht.
Die restlichen Kommentare geben auch keine zusätzlichen Informationen und haben keinen Wert.
Aber zB. eine Beschreibung der Funktionen fehlt.
-
*grml*
genau das was kingruedi gesagt hat wollte ich auch sagen
-
als ergänzung möchte ich noch sagen, dass dieser code stil net aus meiner eigenen feder entsprungen ist, sondern aus einem buch übernommen ist.
das mit dem C vor der klasse hab ich sowieso scho längst verändert, dies ist ein stück source aus dem beispielcode einer einem buch beigefügten CD.
achjo als anmerkung:
is mir eigentlich total latte wie andre leute das finden. solang ich mich selber darin zurecht finde, is das ok, basta.disqualifiziert ihn von jeglicher Teilnahme an einem C++-Code-Style Wettbewerb.
kann mich net erinnern, je an so einem "Wettbewerb", wie du ihn nennst, teilgenommen zu haben. ich hab hier lediglich ein stück code gepostet, wusste net, dass ich damit einen vertrag für einen "Wettbewerb" unterschreibe -.-
[...] typedef LPDINGENSKIRCHEN [...]
naja, immerhin ein versuch, witzig zu sein. kann mich net erinnern, einen typedef gepostet zu haben, und "LPDINGENSKIRCHEN" ... hmja, dazu sag ich jetzt mal gar nix -.-
dass ich BOOL benutze hat schon seinen grund, also maul mich deswegen net an.
da ich nur ein prinzip von code posten wollte, hab ich kommentare net dringelassen, da auch aufgrund des forums eine unübersichtlichkeit in der struktur vonstatten gegangen wäre. wie gesagt, ich will hier keinem erklären was meine klasse macht. das weiss ich zum einen selber und das trägt auch net weiter zum öffentlichen interesse in diesem forum bei.
MFG
#CP.S.:
Tut mir leid dass ich euren "hohen" ansprüchen nicht gerecht bin. ich bin auch nur ein schüler. mir gehts in keinem fall darum, mich in irgendeiner weise mit euch zu konkurrieren; da hab ich noch andre dinge zu tun.kritik nehme ich normalerweise anstandslos hin, aber wenn man andre styles net akzeptieren kann und anscheinend nur der eigene code für einen selber das nonplusultra ist (wie es hier so rüberkommt), dann schwindet jegliches vertrauen in die soziale kompetenz mancher leute...(extrem ausgedrückt)
-
/** Die Klasse Point repräsentiert Koordinaten auf dem * Spielfeld. Diese bestehen aus einer x- und einer y- * Komponente. Von dieser Klasse kann nicht mehr * abgeleitet werden.<br> * Ein Point-Objekt ist <i>unveränderlich</i> (immutable) * und kann deshalb gefahrlos an beliebig vielen Stellen * gleichzeitig referenziert und mit '=' zugewiesen, sowie * an Methoden übergeben werden. * * @author Michael Firbach * @version 1.15 */ public final class Point { /** Konstruiert ein Point-Objekt, indem es die beiden * int-Werte als x- und y- Werte interpretiert. * @param x Der x-Wert der Koordinate. * @param y Der y-Wert der Koordinate. */ public Point(int x, int y) { this.x = x; this.y = y; } /** Konstruktor für ein Point-Objekt. * @param string Ein Punkt in Stringdarstellung. * Zulässig sind die folgenden Schreibweisen:<br><br> * - [Buchstabe]+[Zahl]<br> * - [Zahl]+[Buchstabe]<br><br> * Die Zahl kann aus mehreren Ziffern bestehen, * aber es darf nur ein Buchstabe angegeben sein. * <i>(Eine Feldgröße im Bereich > 15 macht sowieso * keinen Sinn mehr, geschweige denn > 26).</i> * @throws BattleshipException falls der String * nicht als Koordinate interpretiert werden kann. * @see #toString() */ public Point(String string) throws BattleshipException { string = string.toLowerCase().trim(); if( string.length() < 2 ) throw new BattleshipException(); try { // Der Buchstabe kommt zuerst... if( Character.isLetter(string.charAt(0)) ) { y = string.charAt(0) - 'a'; x = Integer.parseInt(string.substring(1)); } // Der Buchstabe kommt zuletzt... else if( Character.isLetter(string.charAt(string.length() - 1)) ) { y = string.charAt(string.length() - 1) - 'a'; x = Integer.parseInt(string.substring(0, string.length() - 1)); } // Es kommt einfach nur noch Mist... else throw new BattleshipException(); } catch( NumberFormatException ex ) { throw new BattleshipException(); } } /** Liefert eine Textdarstellung dieses Punktes, die * für den Menschen gut lesbar ist.<br> Das Format * sieht so aus: [Buchstabe]+[Zahl]<br> * Bsp.: A7, Z8, C5 * @return Eine Textdarstellung dieses Punktes. * @see #Point(String) */ public String toString() { return Character.toString((char)('A' + y)) + (x); } /** Die x-Komponente dieser Koordinate. Ein x-Wert wird * durch Ziffern angegeben und beginnt bei '0', nach * rechts wachsend. * <i>Unveränderlich</i>. */ public final int x; /** Die y-Komponente dieser Koordinate. Ein y-Wert wird * durch Buchstaben angegeben und beginnt bei 'A', * nach unten wachsend. * <i>Unveränderlich</i>. */ public final int y; }Mag ein bisschen übertrieben kommentiert sein, aber war halt für die Schule.
@Zyrian: Nimm dir das halt zu Herzen, was kingruedi gesagt hat. Oder willst du nicht dazulernen?
-
@kingruedi & Shade (der ja der gleichen Meinung ist): Ich bin mir nicht sicher, ob ich euch richtig verstanden habe bzgl. der LP-Typen. Meint ihr, dass man, anstatt z.B. LPDIRECTDRAWSURFACE7 oder was auch immer zu nutzen, sich selbst solch einen Zeiger definieren sollte?
typedef const DirectDrawSurface* LPFOO;oder wie kann man sonst diese lästigen, vordefinierten Typen loswerden?
Über die Einrückung lässt sich streiten (ich persönlich find' sie auch etwas gewöhnungsbedürftig, beim Rest stimme ich euch bei.
-
Zyrian schrieb:
[...] typedef LPDINGENSKIRCHEN [...]
naja, immerhin ein versuch, witzig zu sein. kann mich net erinnern, einen typedef gepostet zu haben, und "LPDINGENSKIRCHEN" ... hmja, dazu sag ich jetzt mal gar nix -.-
Es geht nicht darum, dass du selber typedefs schreibst, sondern diese LPXXX dinger nutzt. Das schlechte daran ist, dass man solche Variablen nicht sofort als Zeiger identifizieren kann. Hat das überhaupt Vorteile?
Zyrian schrieb:
dass ich BOOL benutze hat schon seinen grund, also maul mich deswegen net an.
Ich hab das nie verstanden, warum man sowas macht. Kann mich da mal wer aufklären?
-
Ich nehms lediglich zur Kenntnis. Allerdings scheint Freundlichkeit nicht bei jedem Mitglied des Forums zur persönlichen Grundausstattung zu gehören. Ich lass mich nicht anmaulen, nur weil jemand net mit meinem Codestil zurechtkommt.
Meinen Code für jeden Programmierer speziell zurecht zu fummeln ist das, was mir wirklich von allen Dingen auf der schönen weiten Welt am meisten egal ist.Es geht nicht darum, dass du selber typedefs schreibst, sondern diese LPXXX dinger nutzt. Das schlechte daran ist, dass man solche Variablen nicht sofort als Zeiger identifizieren kann. Hat das überhaupt Vorteile?
Ich wüsst net, dass es sooo verdammt schlimm ist, dass ich LPs benutze.
Aber ihr könnt mir net erzählen, dass ihr LP-Variablen net als Zeiger erkennt. Das kauf ich euch net ab (oder ka was du meinst, du hast dich net ganz verständlich ausgedrückt).btw geht es hier nur um die Deklaration einer Klasse. Die Definitionen sind imho das Entscheidene. Die Deklaration habe ich so gestaltet, dass ich auf einen Blick die wichtigsten Methoden vor den Augen hab und net in unübersichtlichen engen Codezeilen rumsuchen muss.
(Falls es darum geht: Mein Monitor ist keine Leinwand, ist n altes Ding, ich will meine Augen schonen und das tu ich net, indem ich mich durch eng geschriebenen Code wühle, nur um vielleicht eine Parameterliste anzupassen oder sonst was.)Die Methodendefinitionen sehen bei mir "ganz normal" aus, wobei ich wirklich net mehr weiss, was manche unter "normal" verstehen (vielleicht ist für sie nur der eigene Code normal, man weiss es net ...)
-
Ich würde versuchen, von wirklich guten Leuten aus dem C++-Lager zu lernen. Schau dir einfach den Code der Gurus an.
-
Zyrian schrieb:
Ich lass mich nicht anmaulen, nur weil jemand net mit meinem Codestil zurechtkommt.
Meinen Code für jeden Programmierer speziell zurecht zu fummeln ist das, was mir wirklich von allen Dingen auf der schönen weiten Welt am meisten egal ist.Naja, jemand fragt nach guten Stil und du postest so ein WinApi-MFC-Gedöns.
Und die Formatierung, geht kaum noch hässlicherSchlimmer finde ich, dass die Klasse so riesig ist. Da ist der Code bestimmt auch dementsprechend.
Abgesehen davon, dass die ganzen xy- oder rgb-Paare wohl besser als Point oder sonstiges aufgehoben wären, sind die auch noch alle public. Über die get/set-Funktionen, kann man die ändern wie man lustig ist.Alles in allem, nicht gerade die Antwort auf die Überschrift. Da musst du auch Kritik vertragen können
-
Zyrian schrieb:
Es geht nicht darum, dass du selber typedefs schreibst, sondern diese LPXXX dinger nutzt. Das schlechte daran ist, dass man solche Variablen nicht sofort als Zeiger identifizieren kann. Hat das überhaupt Vorteile?
Ich wüsst net, dass es sooo verdammt schlimm ist, dass ich LPs benutze.
Aber ihr könnt mir net erzählen, dass ihr LP-Variablen net als Zeiger erkennt. Das kauf ich euch net ab (oder ka was du meinst, du hast dich net ganz verständlich ausgedrückt).Also ich kann T* deutlich besser als Pointer ausmachen als LPT. Wenn es wenigstens LP_T wäre dann würde man wenigstens auf den ersten Blick sehen, dass es ein Präfix ist.
-
http://www.gotw.ca/
http://www.aristeia.com/
http://www.aristeia.com/Papers/IEEE_Software_JulAug_2004.pdf
etc.
-
Also ich kann mich auch nicht begeistern für den stark auseinandergezogenen Code von zyrian.
Ich denke, du hast es da ein bisschen zu genau genommen mit dieser Art ser Strukturierung, sodass es doch ein wenig schwerer zu lesen ist als du glaubst (du hast dich ja daran schon gewöhnt). Prinzipiell ist eine Strukturierung dieser Art eine gute Idee, nur versuch, die Funktionen so zu gruppieren, dass sie von der Art/Anzahl der Argumente ungefähr zusammenpassen und auf dieser Grundlage die Tabs zu wählen. Wäre etwas leserlicher.
Allerdings ist ein Negativpunkt daran, dass die Funktionen weniger nach gemeinsamen Aufgabenbereich als vielmehr nach optischer Zusammengehörigkeit gruppiert werden, und das ist auch nicht so toll.Was die bescheurerten LPDIRECT... Dinger betrifft, die Kürze ich grundsätzlich durch typedefs oder defines ab. Is mir zuviel zu Schreiben, und die ganzen Großbuchstaben mag ich auch nicht.
-
Optimizer schrieb:
public Point(String string) throws BattleshipException }Hat der Name BattleshipException eine tiefere Bedeutung? Irgendwie find ich den da komisch
-
als ergänzung möchte ich noch sagen, dass dieser code stil net aus meiner eigenen feder entsprungen ist, sondern aus einem buch übernommen ist.
Das macht den Code nicht richtiger, sondern sagt eher was über die Qualität des Buches aus
achjo als anmerkung:
is mir eigentlich total latte wie andre leute das finden. solang ich mich selber darin zurecht finde, is das ok, basta.Da es in dem Thread um guten Stil geht, musst du damit rechnen, dass die Leute deinen Stil kritisieren. Wenn du keine Kritik vertragen kannst, dann poste doch einfach kein Code, dann passiert dir das auch nicht.
Und niemand zwingt dich hier zum lernen...
@ethereal
ich finde typedefs um Pointer zu verstecken keine gute Idee. Also lieber gleichfoo *a; anstelle pfoo *a; schreiben.
(vorallem da das L beim P ja eh keine Bedeutung mehr hat und ein gutes Beispiel für die Sinnbefreitheit der UN ist)
-
Die ganzen LPDIRECTxxx gehören IMHO sowieso in einen CComPtr oder etwas vergleichbares verpackt.
-
KPC schrieb:
Hat der Name BattleshipException eine tiefere Bedeutung? Irgendwie find ich den da komisch
Es handelte sich um ein Schiffe versenken mit cooler Konsolengrafik.