Suche SEHR gutes Beispiel für guten Code-Style

rüdiger

@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.

Shade Of Mine

*grml*

genau das was kingruedi gesagt hat wollte ich auch sagen

Zyrian

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
#C

P.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?

ethereal

@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.

godlikebot

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?

Zyrian

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 ...)

Erhard Henkes

Ich würde versuchen, von wirklich guten Leuten aus dem C++-Lager zu lernen. Schau dir einfach den Code der Gurus an.

DrGreenthumb

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ässlicher

Schlimmer 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.

Erhard Henkes

http://www.gotw.ca/
http://www.aristeia.com/
http://www.aristeia.com/Papers/IEEE_Software_JulAug_2004.pdf
etc.

randa

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.

KPC

Optimizer schrieb:

public Point(String string) throws BattleshipException

}

Hat der Name BattleshipException eine tiefere Bedeutung? Irgendwie find ich den da komisch

rüdiger

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 gleich

foo *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)

tag

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.

otze

Irgendwer schrieb:

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.

nix gegen ein p am anfang um nen pointer anzuzeigen, aber dann muss auch schon klein sein: T* ist für mich genauso aussagekräftig wie pT

rüdiger

@otze
ich finde es unnötig, die eigentliche Syntax der Sprache zu umgehen. Was hat p als Prefix für ein Vorteil? Keinen, also ist es nicht nötig, die Syntax der Sprache zu verstecken, weil es nur zur Verwirrung von Programmierern führen kann.

Ansonsten könnte man ja auch gleich mit entsprechenden defines seine eigene Programmiersprache basteln

Warum nicht auch

#define CALL_PROCEDURE(function,parameter) function(parameter)
#define DECLARE_PROCEDURE(function,parameter) void function(parameter)
#define BEGIN {
#define END }
#define INIT int main()

DECLARE_PROCEDURE(foo,int i);

INIT
BEGIN
  CALL_PROCEDURE(foo,1);
END

oberes macht IMHO genauso viel Sinn, wie Pointer zu verstecken oder ähnliche späße.

Shade Of Mine

kingruedi schrieb:

oberes macht IMHO genauso viel Sinn, wie Pointer zu verstecken oder ähnliche späße.

Aber Microsoft macht es ja auch so. Und die muessen ja wissen wie man programmiert. Sieht man doch an den ganzen Samples aus der MSDn wie gut sie programmieren koennen.

SideWinder

BTW: An den LP-Typedefs kommst du als DirectX-Programmierer nicht vorbei. Und BOOL ist auch besser als bool da ja eventuell in 2 Jahren kein bool mehr dahinter steht (hust, tut es ja jetzt schon nicht...)

MfG SideWinder