Was ist für euch guter Programmcode?
-
Mecnels schrieb:
Und das Game selber koennt ihr dann von meiner Free Webspace Seite, die in meinem Profil steht, einfach herunterladen und ausprobieren, falls ihr immer noch daran zweifelt, dass viele Kommentare und die eine oder andere verschachtelte Schleife einem grossen Programm nur schaden können.
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 10000 LOC an.
-
MaSTaH schrieb:
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 10000 LOC an.
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 100000 LOC an.
-
Gregor schrieb:
MaSTaH schrieb:
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 10000 LOC an.
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 100000 LOC an.
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 1000000 LOC an.
-
Gregor schrieb:
MaSTaH schrieb:
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 10000 LOC an.
LOL, groß? Das ist nicht groß! Groß fängt für mich bei > 100000 LOC an.
Deswegen habe ich ja gesagt "fängt an"
. Ich habe meist keinen Einblick in das komplette Projekt und dementsprechend kann ich auch nicht schätzen wieviele LOC es insgesamt beherbergt. Ich habe nur mal grob geschätzt wieviel in meiner aktuellen Library mittlerweile schon so angefallen sein könnte. Steckt noch in den Kinderschuhen, aber ich würde sie schon jetzt als groß (besser gesagt umfangreich) bezeichnen.
-
// Wird an allen Ecken und Enden fuer Berechnungen benoetigt
Unnötiger Kommentar. Sagt nämlich nichts aus. Ich weiss jetzt nur
Vektor ist wichtig, aber warum es dass ich, weiss ich nicht.
Und was es ist auch nicht.class Vektor{ public: short x,y; Vektor(); Vektor(short xx,short yy); bool operator==(Vektor Vergleich); Vektor operator-(); Vektor operator+(Vektor Summand); Vektor operator-(Vektor Minuend); short operator*(Vektor Faktor1); };
diese operatoren sollten (mit ausnahme des unären -) non member funktionen sein.
// Ein Menge, in der mehrere Vektoren die Elemente sein koennen
Da fällt mir sofort auf: was macht Vecktormenge besser als std::vector ?
Warum kein standard Container?bool AddZug(Vektor Hinzu);
Was macht das bool? true/false bei success fehlschlagen?
Dafür haben wir exception...bool IsInside(Vektor Gesucht);
wäre find() nicht vielleicht besser?
und sollte es nicht vielleicht const sein?warum aufeinmal englisch?
unsigned GetZuegezahl();
wäre das hier const nicht vielleicht besser?
Vektor GetZug(unsigned ZugNummer);
Wäre dafür der op[] nicht besser geeignet? Und const wäre es auch nicht schlecht.
Vektormenge* Zuegearray; // Speichert die Adresse einer Vektormenge // in der die fuer diesen Stein prinzipiell // moeglichen Zuege gespeichert sind
warum ein zeiger?
// eine Feld ist eine Spezialisierung eines Vektors
unnötiger kommentar. Aber was ist ein Feld und _warum_ ist es
die Spezialisierung eines vektors?Piece** Hiesige_Steine;
Wäre hier ein Container nicht angebracht?
Zeiger auf Zeiger sind oft sehr häßlich zu verwenden...HBITMAP* Falls_leer; HBITMAP* Es_geht_ab;
Namen sagen mir garnichts.
// nicht vergessen_baseID wird zum Zwecke der // Zugehoerigkeit zu Spielfeld oder Palettenfeld // vergeben (irgend ein vierstelliger Wert) // BITTE ERST AUFRUFEN, wenn die noetigen Werte // eingegeben wurden (geht leider nur muehsam // und manuell)
Klingt übel und klingt nach refactoring
// Diese Funktion reserviert sogleich auch den // benoetigten Platz fuer die Zeiger auf die Steine // (wobei geplant ist, dass die Zeiger auch auf NULL // zeigen koennen)_Ausserdem zeigen alle Zeiger erstmal // auf NULL, das heisst, man kann nur einmal SetMaximal // aufrufen, um das Programm nicht durcheinander zu bringen
Klingt auch übel. Klingt sehr nach schwer zu findenden Bug wenn man
es doch tut.
Und klingt auch nach: der name SetMaximal passt einfach nicht.// Diese Funktion nimmt quasi den Stein von ganz oben vom // Stapel, d.h. setzt erstens den entsprechenden Zeiger auf // NULL, reduziert 'aktuell' wieder um 1 (sofern moeglich, // falls nicht: NULL-Zeiger retour. (!!! Wird der Rueckgabewert // nicht gespeichert, geht dem Spiel praktisch ein Spielstein // verloren !!! - UebernimmStein und SteinWegnehmen arbeiten wie // ein FILO Stapel zusammen)
Klingt echt kompliziert und bug anfällig.
// WENN ALLES KORREKT INITIALISIERT WURDE, DANN KANN ENDLICH // ZUM ERSTEN MAL DIE FUNKTION Draw() aufgerufen werden, // natuerlich weiss das Feld selbst, was wohin gezeichnet wer- // den soll, sonst haette ich mir ja die Mordsinitialisierungs- // funktion der Klasse MTactics gleich schenken koennen void Draw(bool spielt_sich_ab=false);
Der Parameter ist mir nicht klar
Und ein enormer Aufwand bis ich Draw() aufrufen kann, ist das auch.Sollte man vielleicht etwas vereinfachen...
// INNERHALB DER ANWENDUNGSKLASSE SPIELT SICH PRAKTISCH ALLES // AB, SIE IST DIE EIGENTUEMERIN UND OBERSTE VERWALTERIN DES // GESAMTEN PROGRAMMS, ein riesiger Container mit wenigen, // ziemlich grossen Funktionen, die dafuer aber auch entsprechend // maechtig sind
Hilft mir nicht viel um zu verstehen was da abgeht.
Wenn ich eine Klasse sehe, gehe ich immer davon aus, dass sie wichtig ist...
HINSTANCE hinzz; // Von Create erledigt HBITMAP verwendet[28]; // Von Create erledigt Vektormenge Zuegearray[3]; // Von Create erledigt Piece Steine[80]; // Von Create erledigt Feld Spielfeld[10][10],Palettenfeld[3][4]; // Von Create erledigt HWND vergrPlr,vergrCpu,plrText,cpuText; // Von Create erledigt HWND Startbutton,Palbutton,Qustartbutton; // Von Create erledigt HWND hauptfenster; // Von Create erledigt char stnamen[12][12]; // Von Create erledigt char no_selection[5]; // Von Create erledigt Vektor hilfsvektorarray[2][210]; // in [0] werden die Start- // felder gespeichert // in [1] die Zugehoerigen // Zielfelder short Spielphase; // 1 fuer Aufstellphase, 2 fuer Spielphase short NextPhase; // Mitunter benoetigt Piece* plrchosen; // In diesen Variablen werden Spielsteine Piece* cpuchosen; // gespeichert, die gerade ausgewaehlt // wurden Piece* winnerx; // Hier wird der Gewinner einer Begegnung zwischengespeichert Vektor Startfeld; // hier wird gespeichert, von wo der Spieler gestartet // ist, um spaeter einen Zug auf seine Zulaessigkeit // pruefen zu koennen. Falls -1/-1, dann gibt es noch kein // Startfeld Vektor Zielfeld; // Zur Zwischenspeicherung gedacht bool fuenfunbekannt; // Eine Variable, die die Fairness der KI sicherstellen // soll public: bool DoFast; // noetig, um die Bedienung des Games mit der Maus // etwas komfortabler zu gestalten WPARAM reserve;
Bo ey. Soviele Variablen, teils Zeiger teils nicht. Und ein paar wahnsinns arrays.
Ne, die Klasse will ich nicht warten müssen...bool Create(HINSTANCE instanz,HWND hauptf);
Warum eigentlich immer Create() und nicht im Ctor?
// Anzeigefunktion, die das gesamte Game auf dem Bildschirm abbilden kann, // wird aber nur ein einziges Mal, und zwar von Create, aufgerufen bool ShowAll();
Was liefert sie denn zurück? Und warum ist sie public wenn nur Create sie aufrufen darf?
// Kuerzt die Aufstellprozedur ab bool QuickStart();
??
sagt mir garnix. und was macht das bool?// Stelle Deine Figuren auf _ nach einem streng geheimen (lol) Algorithmus // stellt hier der Computer seine Steine auf, oder auch die des // Spielers, wenn der auf Quickstart geklickt hat bool CpuAufstellen(short addf=40);
wasmacht addf?
// Computer_zieht() befasst sich mit der Berechnung des Computerzuges // das ist de facto die AI - Funktion, hat aber die beiden Helferlein // StrongAI() und FleeAI(), die sie unterstuetzen void Computer_zieht();
Eine ganze AI in 3 funktionen... Ist sicher riesig.
// Diese Funktion ueberprueft, ob die Aufstellung des Spielers zumindest // das Eis enthaelt, nur dann // wird das Spiel ueberhaupt gestartet, klasse Beispiel fuer Redundanz // extra kurze Funktion die so gut wie nie gebraucht wird bool Checkvalid();
Eis?
#define SPIELFELDID 1000 #define PALETTENFELDID 2000 #define PALBUTTONID 3000 #define VERGRCPUID 4000 #define VERGRHUMID 5000 #define VERGRPLRID 5000 #define TEXTCPUID 6000 #define TEXTPLRID 7000 #define TEXTSIZE 33 #define STARTBUTTONID 8000 #define EINZELFELDGROESSE 48 #define HUMPLR 1 #define CPUPLR 2 #define LARGESIZE 130 #define BUTTONSIZE 100 #define AUFSTELLZONE 6 #define QUICKSTARTID 9000
Schonmal was von const gehört?
-
Shade Of Mine schrieb:
diese operatoren sollten (mit ausnahme des unären -) non member funktionen sein.
Jetzt enttäuscht Du mich aber ein bisschen. Erklär mir bitte wie Du einen Skalar zu einem Vektor hinzuaddieren willst? Das ist gar nicht definiert, und um zu verhindern, dass Konvertierungskonstruktoren hier Unsinn anrichten können, sollte man gerade bei Vektoraddition oder Subtraktion KEINE globalen Funktionen definieren, sondern ausschließlich Elementfunktion. Das ist ja so elementar. Globale Operatoren machen zum Beispiel bei Klassen für komplexe oder rationale Zahlen Sinn, weil hier tatsächlich des öfteren Ganzzahlen zu Dupeln addiert werden, sodass Konvertierungskonstruktoren auch keinen Schaden anrichten können. Aber sowas wie Ganzzahl + Vektor ist total unsinnig und rechtfertigt in jedem Fall die Implementierung der Operators + als Elementfunktion.
-
Hallo,
wenn du nicht willst, dass eine Ganzzahl implizit zu einem Vektor wird, dann mach die Konstruktoren explicit.
Sprich: wenn du Angst vor den Auswirkungen eines Konvertierungsctors hast, dann sorg dafür, dass es keinen solchen Ctors gibt. C++ hat nicht umsonst dafür extra ein Schlüsselwort.
Irgendwie tendierst du dazu die Pferde von der falschen Seite aufzuzäumen.
-
Dann werf ich euch Haien auch noch ein Stücklein Code zum Frass vor.
Die Klasse, in Java, interpretiert Elemente eines Textes, als wären es Componenten (Buttons + etc.) auf einer GUI, und verschickt an spezielle Listener Events, wenn der Benutzer mit der Maus rumspielt.
(Aufmerksame Beobachter mögen erkennen, dass der Code noch nicht sehr alt ist, einige Kommentare fehlen noch)
Was mich bewegt den Code zu posten? Ich denke er ist gut genug geschrieben, dass man ihn an verschiedenen Orten einsetzen kann, und auch eine Erweiterung möglich ist.
[Edit: und ich bin interessiert, was ihr davon haltet]/* * Created on 09.01.2005 */ package bibliothek2.gui.text; /** * Dieser Listener wird einem {@link bibliothek2.gui.text.EditorGuard EditorGuard} * hinzugefügt, und von dem Guard von Veränderungen unterrichtet.<br> * Die Methoden dieses Interfaces sind dem {@link java.awt.event.MouseListener MouseListener} * und dem {@link java.awt.event.MouseMotionListener MouseMotionListener} * nachempfunden, und werden auch entsprechend genutzt. * * @author Benjamin Sigg */ public interface EditorListener{ public void mouseMoved( EditorEvent event ); public void mouseClicked( EditorEvent event ); public void mouseExited( EditorEvent event ); public void mouseEntered( EditorEvent event ); public void mouseDragged( EditorEvent event ); public void mousePressed( EditorEvent event ); public void mouseReleased( EditorEvent event ); }
/* * Created on 09.01.2005 */ package bibliothek2.gui.text; import java.awt.event.MouseEvent; import javax.swing.text.Element; /** * Ein EditorEvent dient als Übertragung von Informationen von einem * {@link EditorGuard} zu einem {@link EditorListener}. * * @author Benjamin Sigg */ public class EditorEvent{ /** * Information, wohin dieses Event ursprünglich geschickt wurde. * Z.b. sollte ein CLICKED-Event an {@link EditorListener#mouseClicked(EditorEvent)} * geschickt werden. */ public enum Typ{ CLICKED, EXITED, ENTERED, MOVED, DRAGGED, PRESSED, RELEASED }; private Typ typ; private MouseEvent origin; private EditorGuard source; private Element element; /** * Standardkonstruktor * @param source Der Guard, welcher das Event auslöst * @param element Das Element, welches betroffen ist * @param origin Das MouseEvent, welches zu dem verschicken dieses Eventes * geführt hat * @param typ Der Typ des Events */ public EditorEvent( EditorGuard source, Element element, MouseEvent origin, Typ typ ){ this.typ = typ; this.origin = origin; this.source = source; this.element = element; } /** * Gibt das Element zurück, für welches das Event geworfen wurde. * @return das Element */ public Element getElement() { return element; } /** * Gibt das MouseEvent zurück, welches zum Auslöser dieses Events wurde * @return Der Auslöser */ public MouseEvent getOrigin() { return origin; } /** * Gibt den EditorGuard zurück, welcher dieses Event verschickt hat. * @return Der Gaurd */ public EditorGuard getSource() { return source; } /** * Gibt den Typ dieses Eventes zurück. * @return Der Typ */ public Typ getTyp() { return typ; } public String toString(){ return getClass().getName() + " [typ= " + typ + ", element= " + element + ", source= " + source + ", origin= " + origin + "]"; } }
/* * Created on 09.01.2005 */ package bibliothek2.gui.text; import java.awt.Point; import java.awt.Rectangle; import java.awt.Shape; import java.awt.event.MouseEvent; import java.awt.event.MouseListener; import java.awt.event.MouseMotionListener; import java.util.Vector; import javax.swing.JEditorPane; import javax.swing.plaf.TextUI; import javax.swing.text.BadLocationException; import javax.swing.text.Document; import javax.swing.text.Element; import javax.swing.text.Position; /** * Der EditorGuard überwacht die Mausbewegungen über einem * {@link javax.swing.JEditorPane JEditorPane} und verschickt Events, sollte * der Benutzer ein {@link javax.swing.text.Element Element} klichen, es * überfahren, oder andere Mausaktionen machen.<br> * Der EditorGuard kann immer nur ein EditorPane überwachen. * * @see bibliothek2.gui.text.EditorListener EditorListener * @see bibliothek2.gui.text.EditorEvent EditorEvent * * @author Benjamin Sigg */ public class EditorGuard{ private Element element; private JEditorPane editor; private Listener listener; private Vector<EditorListener> listeners = new Vector<EditorListener>(); public EditorGuard(){ listener = createListener(); } public EditorGuard( JEditorPane editor ){ this(); setEditor( editor ); } public void addEditorListener( EditorListener listener ){ listeners.add( listener ); } public void removeEditorListener( EditorListener listener ){ listeners.remove( listener ); } /** * Setzt den zu überwachenden Editor. * @param editor Der neue Editor */ public void setEditor( JEditorPane editor ){ if( this.editor != null ){ this.editor.removeMouseListener( listener ); this.editor.removeMouseMotionListener( listener ); } this.editor = editor; if( editor != null ){ editor.addMouseListener( listener ); editor.addMouseMotionListener( listener ); } listener.reset(); } /** * Gibt das JEditorPane zurück, welches von diesem Guard überwacht wird. * @return Der Editor oder null, sollte kein Editor gesetzt sein */ public JEditorPane getEditor(){ return editor; } /** * Generiert einen Listener. Dieser Listener wird den JEditorPanes hinzugefügt. * @return Der neue Listener */ protected Listener createListener(){ return new Listener(); } /** * Gibt dasjenige Element zurück, auf das sich derzeit alle verschickten * Events beziehen. * @return Das Element, auf das sich alle Events beziehen. */ public Element getElement(){ return element; } /** * Setzt das Element, auf das sich alle durch <code>fireX</code> verschickten * Events beziehen. * @param element Das Element */ protected void setElement( Element element ){ this.element = element; } /** * Nicht jedes Element ist interessant genug um ein Event zu verschicken. * Diese Methode entscheidet, für welche Elemente Events versendet werden. * @param element Das zu prüfende Element * @return true, wenn Events dieses Element betreffend verschickt * werden soll */ protected boolean isInteresting( Element element ){ return true; } /** * Gibt dasjenige Element zurück, welches auf dem JEditorPane dieses * Guards, an den angegebenen Koordinaten x/y liegt. * @param x Die x-Koordinaten * @param y Die y-Koordinaten * @return Das Element oder null, sollte sich an den angegebenen Koordinaten * kein Element befinden, oder falls kein Editor gesetzt ist. */ public Element getElement( int x, int y ){ if( editor == null ) return null; if( !editor.getVisibleRect().contains( x, y ) ) return null; Document document = editor.getDocument(); Point location = new Point( x, y ); int offset = editor.viewToModel( location ); Element element = getElement( offset, document ); if( elementContainsLocation( editor, element, location )) return element; else return null; } /** * Sucht in dem Document dasjenige Element, dessen Grenzen das offset * umschliessen.<br> * Die Defaultimplementation untersucht dazu nur das DefaultRootElement. * @param offset Die gesuchte Position im Document * @param document Das zu untersuchende Document * @return Das Element */ public Element getElement( int offset, Document document ){ Element element = document.getDefaultRootElement(); while( !element.isLeaf() ){ int index = element.getElementIndex( offset ); element = element.getElement( index ); } return element; } /** * Überprüft ob an dem Punkt <code>location</code> auf dem JEditorPane * <code>editor</code> das Element <code>element</code> angezeigt wird. * @param editor Der Editor, auf dem das Element angezeigt wird * @param element Das Element * @param location Der Punkt im Koordinatensystem von editor * @return true, wenn das Element den Punkt beinhaltet, false andernfalls */ public boolean elementContainsLocation( JEditorPane editor, Element element, Point location ){ try { TextUI ui = editor.getUI(); Shape begin = ui.modelToView( editor, element.getStartOffset(), Position.Bias.Forward ); if ( begin == null) { return false; } Rectangle bounds = (begin instanceof Rectangle) ? (Rectangle)begin : begin.getBounds(); Shape end = ui.modelToView( editor, element.getEndOffset(), Position.Bias.Backward); if (end != null) { Rectangle endBounds = (end instanceof Rectangle) ? (Rectangle)end : end.getBounds(); bounds.add( endBounds ); } return bounds.contains( location.x, location.y ); } catch (BadLocationException ex) { return false; } } /** * Schickt dieses Event an die dafür vorgesehene Methode der Listener ab. * @param event Das zu verschickende Event */ protected void fireEvent( EditorEvent event ){ EditorListener[] listeners = this.listeners.toArray( new EditorListener[ this.listeners.size() ] ); EditorEvent.Typ typ = event.getTyp(); if( typ == EditorEvent.Typ.CLICKED ) for( EditorListener listener : listeners ) listener.mouseClicked( event ); else if( typ == EditorEvent.Typ.DRAGGED ) for( EditorListener listener : listeners ) listener.mouseDragged( event ); else if( typ == EditorEvent.Typ.ENTERED ) for( EditorListener listener : listeners ) listener.mouseEntered( event ); else if( typ == EditorEvent.Typ.EXITED ) for( EditorListener listener : listeners ) listener.mouseExited( event ); else if( typ == EditorEvent.Typ.MOVED ) for( EditorListener listener : listeners ) listener.mouseMoved( event ); else if( typ == EditorEvent.Typ.PRESSED ) for( EditorListener listener : listeners ) listener.mousePressed( event ); else if( typ == EditorEvent.Typ.RELEASED ) for( EditorListener listener : listeners ) listener.mouseReleased( event ); else throw new IllegalArgumentException( "unknown typ: " + typ ); } /** * Ruft die {@link EditorListener#mouseClicked(EditorEvent) mouseClicked}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMouseClicked( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.CLICKED ) ); } /** * Ruft die {@link EditorListener#mouseMoved(EditorEvent) mouseMoved}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMouseMoved( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.MOVED ) ); } /** * Ruft die {@link EditorListener#mouseExited(EditorEvent) mouseExited}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMouseExited( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.EXITED ) ); } /** * Ruft die {@link EditorListener#mouseEntered(EditorEvent) mouseEntered}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMouseEntered( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.ENTERED ) ); } /** * Ruft die {@link EditorListener#mouseDragged(EditorEvent) mouseDragged}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMouseDragged( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.DRAGGED ) ); } /** * Ruft die {@link EditorListener#mousePressed(EditorEvent) mousePressed}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMousePressed( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.PRESSED ) ); } /** * Ruft die {@link EditorListener#mouseReleased(EditorEvent) mouseReleased}-Methode * aller registrierter EditorListener auf. * @param origin Das Event, welches die Aktion auslöste */ protected void fireMouseReleased( MouseEvent origin ){ fireEvent( new EditorEvent( this, element, origin, EditorEvent.Typ.RELEASED ) ); } /** * Der Listener überwacht die Bewegung der Maus, und leitet die Events * weiter. */ protected class Listener implements MouseListener, MouseMotionListener{ /** * Die Anzahl gedrückter Maustasten<br> * Wenn diese Zahl grösser als 0 ist, sollten sich alle Events * auf das gespeicherte Element beziehen, und es sollte kein * neues Element untersucht werden. */ protected int pressCount = 0; /** * Das Element das sich unter der Maus befindet, während sie im * "drag"-Zustand ist. */ protected Element dragElement = null; /** * Wird von dem Guard aufgerufen wenn ein neues JEditorPane gesetzt * wird. Der Listener sollte danach wieder ein Verhalten haben, als * wäre keine Maustaste gedrückt.<br> * Wird die Methode überschrieben, wird ein Aufruf durch * <code>super.reset()</code> empfohlen. */ public void reset(){ pressCount = 0; } public void mouseClicked( MouseEvent e ) { Element element = getElement( e.getX(), e.getY() ); setElement( element ); if( element != null && isInteresting( element )) fireMouseClicked( e ); } public void mousePressed( MouseEvent e ) { if( pressCount == 0 ){ element = getElement( e.getX(), e.getY() ); dragElement = element; } pressCount++; if( element != null && isInteresting( element )) fireMousePressed( e ); } public void mouseReleased( MouseEvent e ) { pressCount = Math.max( 0, pressCount-1 ); if( element != null && isInteresting( element )) fireMouseReleased( e ); if( pressCount == 0 ){ Element newElement = dragElement; if( newElement != element ){ if( element != null && isInteresting( element )) fireMouseExited( e ); setElement( newElement ); if( element != null && isInteresting( element )) fireMouseEntered( e ); } } } public void mouseEntered( MouseEvent e ) { if( pressCount == 0 ){ setElement( getElement( e.getX(), e.getY() ) ); if( element != null && isInteresting( element )) fireMouseEntered( e ); } else{ dragElement = getElement( e.getX(), e.getY() ); if( dragElement != null && isInteresting( dragElement )) fireEvent( new EditorEvent( EditorGuard.this, dragElement, e, EditorEvent.Typ.ENTERED )); } } public void mouseExited( MouseEvent e ) { if( element != null && isInteresting( element )) fireMouseExited( e ); if( pressCount == 0 ) setElement( null ); else{ if( dragElement != null && isInteresting( dragElement )) fireEvent( new EditorEvent( EditorGuard.this, dragElement, e, EditorEvent.Typ.EXITED )); dragElement = null; } } public void mouseDragged( MouseEvent e ) { Element newElement = getElement( e.getX(), e.getY() ); if( element != null && isInteresting( element )) fireMouseDragged( e ); if( newElement != dragElement ){ if( dragElement != null && isInteresting( dragElement )){ fireEvent( new EditorEvent( EditorGuard.this, dragElement, e, EditorEvent.Typ.EXITED )); } dragElement = newElement; if( dragElement != null && isInteresting( dragElement )) fireEvent( new EditorEvent( EditorGuard.this, dragElement, e, EditorEvent.Typ.ENTERED )); } } public void mouseMoved( MouseEvent e ) { Element newElement = getElement( e.getX(), e.getY() ); if( newElement != element ){ if( element != null && isInteresting( element )){ fireMouseMoved( e ); fireMouseExited( e ); } setElement( newElement ); if( element != null && isInteresting( element )){ fireMouseEntered( e ); fireMouseMoved( e ); } } else if( element != null && isInteresting( element )) fireMouseMoved( e ); } } }
Guten Apetit
-
Jetzt mal eine ehrlich gemenínte Frage: habt ihr jetzt nur kommentare hinzugefügt oder standen die vo anfang an da. Wenn das nämlich so ist sollte ich meinen "Kommentierungsstil" schnell ändern...
-
Mecnels schrieb:
Das ist gar nicht definiert, und um zu verhindern, dass Konvertierungskonstruktoren hier Unsinn anrichten können, sollte man gerade bei Vektoraddition oder Subtraktion KEINE globalen Funktionen definieren, sondern ausschließlich Elementfunktion. Das ist ja so elementar. Globale Operatoren machen zum Beispiel bei Klassen für komplexe oder rationale Zahlen Sinn, weil hier tatsächlich des öfteren Ganzzahlen zu Dupeln addiert werden, sodass Konvertierungskonstruktoren auch keinen Schaden anrichten können. Aber sowas wie Ganzzahl + Vektor ist total unsinnig und rechtfertigt in jedem Fall die Implementierung der Operators + als Elementfunktion.
Hume hat es perfekt erklärt warum dein Ansatz etwas ungewöhnlich ist.
Und was sagst du zu meiner anderen Kritik? Oder hast du gute Argumente warum explicit hier beim Ctor nicht angebracht wäre, etc.
Ich bin gerne bereit offen zu diskutieren.
-
ness schrieb:
Jetzt mal eine ehrlich gemenínte Frage: habt ihr jetzt nur kommentare hinzugefügt oder standen die vo anfang an da. Wenn das nämlich so ist sollte ich meinen "Kommentierungsstil" schnell ändern...
Ich schreibe wenig Kommentare, aber viel Dokumentation. Das ist einfach schlechte Erfahrung mit zuwenig Dokumentation... das dort oben ist mein üblicher Dokumentierstil.
-
ness schrieb:
Jetzt mal eine ehrlich gemenínte Frage: habt ihr jetzt nur kommentare hinzugefügt oder standen die vo anfang an da. Wenn das nämlich so ist sollte ich meinen "Kommentierungsstil" schnell ändern...
Dokumentieren tue ich eigentlich immer ganz gut, wenn ich nicht gerade besonders schlecht gelaunt bin. Meisten reifen die Doc-Kommentare dann auch erst im Laufe der Zeit richtig aus, weswegen z.B. Shade gleich mal entdeckt hat, dass mein file kein Dateiname, sondern ein Pfad zu einer Datei ist.
Mit der Zeit finde ich solche Sachen dann und bessere sie aus.Kommentare im Source findet man bei mir eh nur spärlich, ich hab jetzt sogar einige noch raus, seit dem ich das gepostet habe - zu trivial.
-
also das einzige was mich an mecnels code wirklich stört ist das er deutsch und englisch mischt. das macht einen ja kirre wenn man sich das anguckt
und zu JBeni
/** * Gibt dasjenige Element zurück, auf das sich derzeit alle verschickten * Events beziehen. * @return Das Element, auf das sich alle Events beziehen. */ public Element getElement(){ return element; } /** * Setzt das Element, auf das sich alle durch <code>fireX</code> verschickten * Events beziehen. * @param element Das Element */ protected void setElement( Element element ){ this.element = element; } /** * Nicht jedes Element ist interessant genug um ein Event zu verschicken. * Diese Methode entscheidet, für welche Elemente Events versendet werden. * @param element Das zu prüfende Element * @return true, wenn Events dieses Element betreffend verschickt * werden soll */ protected boolean isInteresting( Element element ){ return true; }
die kommentare von get und setElement sind irgendwie unnötig, sollte selbstsprechend sein. und eigentlich kannst du element ja public machen, es wird doch eh nichts überprüft.
bei isInteresting fehlt wohl noch ein "// todo!!!" oder?
-
thx
borg schrieb:
die kommentare von get und setElement sind irgendwie unnötig, sollte selbstsprechend sein. und eigentlich kannst du element ja public machen, es wird doch eh nichts überprüft.
Na, 1. sind public Variablen hässlich (man kann den Aufbau nur noch schwer ändern), 2. kann nicht jeder das Element setzen, der Setter ist protected (ich finde nichts schlimmer als private, das hat so den Geschmack "stop, ich bin nicht erweiterbar, stop, mein Verhalten ist festgelegt, stop", aber protected Variablen gefallen mir auch nicht...).
Hm, jaa, notwendig sind sie wirklich nicht, aber ohne sehen die Methode so nackt aus...borg schrieb:
bei isInteresting fehlt wohl noch ein "// todo!!!" oder?
Das war Absicht, wenn mal jemand nur an Elementen die z.B. HTML-Links sind, interessiert ist, kann er so ganz einfach eine Unterklasse die nur auf HTML-Links reagiert schreiben.
Das sollte ich wohl in die Doku schreiben.
-
Der Thread hat mir gezeigt das kommentare deutlich wichtiger sind als ich bisher dachte...
Ich habe mal einen unvollständigen code von mir rausgezerrt und die deklaration kommentiert:#pragma once #include <TypeTraits.h> #include <cassert> #include <vector> #include <algorithm> #include <fstream> #include <ctime> #include <cstdlib> #include <string> #include <sstream> //genetic_algorithm.h - entält eine über templates möglichst konfigurierbare Klasse für genetische Algorithemn sowie Standardvorgaben //Hier stehen die Standardvorgaben in Form von Funktionen, nicht gezeigt template < class individual, //Klasse, die die Individuen repräsentiert; Individue werden immer mit new erzeugt und mit delete gelöscht class evaluation_type, //Typ, mit dem die Bewerungsfunktionen arbeiten; Annahme: keine hohen kopierkosten unsigned how_much, //Startmenge der Population evaluation_type (*evaluator)(individual&), //Bewerungsfunktion bool (*is_better)(typename Loki::TypeTraits<evaluation_type>::ParameterType,typename Loki::TypeTraits<evaluation_type>::ParameterType), //Vergleichsfunktion für Individuen evaluation_type perfect, //Wenn die Bewrtungsfunktion diesen Wert zurückgibt, wird das individuum als perfekt angesehen individual* (*generator)(), //generiert zufällige Individuen individual* (*select_to_recombine)(std::vector<std::pair<individual*,evaluation_type> >&), //Wählt aus dem übergebenen Vektor ein zu rekombiniernendes Individuum aus //(wird also immer mindestens paarweise aufgerufen) individual* (*select_to_mutate)(std::vector<std::pair<individual*,evaluation_type> >&), //Wählt aus dem übergebenen Vektor ein zu mutiernedes Individuum aus individual* (*mutator)(individual&), //mutiert individuen; es wird ein neues individuum zurückgegeben individual* (*recombine)(individual&,individual&), //rekombiniert zwei individuen; es wird ein neues zurückgegeben unsigned (*mutate_count)(unsigned), //wieviele Individuen sollen mutiert werden? übergeben wird die größe der Population nach der Bewertung unsigned (*recombine_count)(unsigned), //dito, für rekombinierende Individuen void(*selector)(std::vector<std::pair<individual*,evaluation_type> >&,std::vector<std::pair<individual*,evaluation_type> >&,std::vector<std::pair<individual*,evaluation_type> >&), //übergeben werden die Ausgangsindividuen, die rekombinierten Individuen und die mutierten Individuen; die Ausgangsmenge ist mit einer neuen Ausgangsmenge zu befüllen; //die vektoren aus mutierten und rekombinierten Individuen werden nach dem Aufruf dieser Funktion gelöscht bool verbose > class genetic_algorithm { private: //das beste derzeit vorhandene Individuum, Element von Indivs std::pair<individual*,evaluation_type>best; //die Aktuellen Individuen std::vector<std::pair<individual*,evaluation_type> > indivs; //Hilfsfunktion - führt eine komplette Runde durch (Bewerten, Mutieren, Rekombinieren, Selektiern, Aktualisieren) //aktualisiert perfect_found; gibt perfect_found zurück bool one_time(); bool perfect_found; //Hilfsfunktion - bewertet den gesamten Vektor, füllt evaluation_type aus void evaluate(std::vector<std::pair<individual*,evaluation_type> >& input); //wenn verbose gesetzt ist, wird hier die Anzahl bereits duchgeführter Runden gezählt unsigned counter; genetic_algorithm(const genetic_algorithm&); //noch nicht implementiert, wird irgendwann public const genetic_algorithm& operator=(const genetic_algorithm&); //dito public: std::pair<individual*,evaluation_type> get_best() {if(best.first==0)throw std::runtime_error("Not available!");else return best;}; void reset(); bool perfect_indiv_found()const{return perfect_found;}; //arbeitet den Algo so lange ab, bis ein Individuum mit Gefunden ist, für das die Bewertungsfunktion finish zurückgibt; //führt nicht mehr als count runden aus, wenn count 0 ist wird solange gearbeitet bis Bedingung 1 erfüllt ist individual *start(evaluation_type finish,unsigned count); ///arbeitet den Algo count Runden ab oder bis ein perf. indiv. gef. wurde; wenn count=0 solange bis ein perfektes indiv gefunden wurde individual *do_for(unsigned count); //arbeitet den Algo so lange ab wie ein Aufruf von pwhile true zurückgibt oder ein perfektes Individuum gefunden wurde - hilfreich bei multithreading??? individual *do_while(bool(*pwhile)()); //arbeitet den Algo so lange ab, bis ein perfektes Individuum gefunden wurde -> mit Vorsicht zu genießen individual *do_until_perfect_found(); genetic_algorithm(); ~genetic_algorithm(); std::string print()const; std::vector<std::pair<individual*,evaluation_type> >&get_impl_ref(){return indivs;}; unsigned get_counter()const {if(!verbose)throw std::runtime_error("Verbose-flag not set!");else return counter;}; std::ostream& inner_operator_out(std::ostream& o); //mein compiler meckert wenn ich versuche das über friends zu lösen std::istream& inner_operator_in(std::istream& o); //dito };
Ich versuche halt immer möglichst aussagekräftige Namen zu benutzen um mich um die Kommentare zu drücken...
-
ness: Schreib deine Kommentare im Template nicht einfach hintendran, sondern davor, weil du sonst zu weit in die Breite gehst. Dann würd ich noch ne menge Leerzeilen reinbauen (z.B. immer nach nem Kommentar mit ner Funktionsdeklaration). Dann noch ein paar Leerzeichen (hinter //, zwischen () und const).
Inhaltlich hab ichs mir noch nicht angeguckt.
-
Shade Of Mine schrieb:
Und was sagst du zu meiner anderen Kritik? Ich bin gerne bereit offen zu diskutieren.
So, dann werde ich eben Schlüsse aus der Kritik ziehen.
1.) Verwende keine defines, denn es ist nicht sinnvoll, das gesamte äußere Erscheinungsbild einer Anwendung einfach dadurch verändern zu können, dass man den symbolischen Konstanten andere Werte gibt. Besser ist es, an hundert verschiedenen Positionen irgendwo in hunderten Modulen die tatsächlichen Werte hinzuschreiben, denn wenn man dann dahinter kommt, dass die Textgroesse doch um 2 logische Einheiten größer sein soll, dann hat man das Vergnügen, dutzende Dateien durchsuchen zu müssen und den Wert eben hundertmal ausbessern zu dürfen, und das gleiche nochmal, wenn doch die kleiner Textgröße gereicht hätte. So schlägst du wenigstens Stunden an Zeit tot, was zwar mit define nicht nötig gewesen wäre (da hätte es gereicht den einen Wert daneben auszubessern), aber wie auch immer.
2.) Funktionen sollten ihrem Aufrufer generell keine Auskunft darüber geben, ob während ihrer Ausführung irgend welche Fehler aufgetreten sind, darum sollte man vor allem keine bool Werte zurückgeben, denn dann könnte man ja womöglich den Fehler bei Probeläufen sofort entdecken. Es macht einfach mehr Spaß, in einem großen Programm unter hunderten schweigenden Minifunktionen einen Fehler finden zu müssen.
3.) Verwende keine Zeiger auf Zeiger, denn ... warum eigentlich? Ach ja, das spart nämlich sehr viel Speicher und das ist <template Begruendung bitte> auch nicht gut.
4.) Als Elementfunktionen überladene arithmetische Operatoren sind nicht gut. Die sind nämlich zu wenig fehleranfällig, besser du überlädst sie global und lässt dich davon überraschen, was unter Mitwirkung Deiner Konvertierungskonstruktoren oft an interessanten Ergebnissen dabei heraus kommt. Ist einfach zu lustig (ach und explicit ist zwar alles andere Plattformunabhängig, aber wer braucht das schon bei Klassen für mathematische Objekte? Sind ja sowieso alle Plattformspezifisch).
------
Aus dem besagten Header wurde schliesslich ein Hauptprogramm, an dessen
Bezeichnern alleine man erkennt, was das Programm macht#include"MTdef.h" HINSTANCE instanz; MTactics DasSpiel; // Endlich - die gesamte Anwendung in ein Objekt gepackt LRESULT CALLBACK MessageHandler(HWND hwnd,UINT imsg, WPARAM wParam,LPARAM lParam){ switch(imsg){ case WM_CREATE: if(!DasSpiel.Create(instanz,hwnd)) MessageBox(NULL,"Fehler","Fehler",0); break; case WM_PAINT: DasSpiel.ShowAll(); break; case WM_COMMAND: DasSpiel.ProcessClick(wParam); if(DasSpiel.DoFast){ DasSpiel.DoFast=false; DasSpiel.ProcessClick(DasSpiel.reserve); } return 0; case WM_DESTROY: PostQuitMessage(0); return 0; } return DefWindowProc(hwnd,imsg,wParam,lParam); } int WINAPI WinMain(HINSTANCE hinst,HINSTANCE phinst, LPSTR commline,int showstyle){ // usw usf. - das Übliche das hier steht
// Dieses Programm ist selbsterklärend, wie ich denke.
// Alles was passiert, ist, dass das Spiel erzeugt, mitunter neu gezeichnet
// wird, und schliesslich dass es Benutzer Eingaben beliebigen Typs zu ver-
// arbeiten imstande ist. Genau wie jedes andere Programm auch.--- Ein Projekt kann man frontal angehen, also einfach drauflosprogrammieren,
--- wenn einem der Code in einer Funktion zu lang wird, einen Funktionsnamen
--- dafür erfinden, weiterprogrammieren, wieder einen Funktionsnamen erfinden
--- und so weiter und so fort. Irgendwann hat man dann hunderte Funktionen,
--- wahrscheinlich auch eine Lösung für das Problem, und kann sogar von
--- Modularisierung sprechen.--- Oder man kann es aus simplen Bausteinen wie etwa Vektoren, Feldern,
--- Spielsteinen etc., die alle Elemente des Programmes sind, allmählich zu
--- einem Ganzen zusammenfügen. Und letzteres heisst dann objektorientiert.--- Gute Nacht
-
Bitte hör auf. Das ist ja schrecklich.
-
MecnelsNichtEingeloggt schrieb:
1.) Verwende keine defines, denn es ist nicht sinnvoll, das gesamte äußere Erscheinungsbild einer Anwendung einfach dadurch verändern zu können, dass man den symbolischen Konstanten andere Werte gibt. Besser ist es, an hundert verschiedenen Positionen irgendwo in hunderten Modulen die tatsächlichen Werte hinzuschreiben
Verarscht du mich?
2.) Funktionen sollten ihrem Aufrufer generell keine Auskunft darüber geben, ob während ihrer Ausführung irgend welche Fehler aufgetreten sind, darum sollte man vor allem keine bool Werte zurückgeben,
Weil es Exception gibt?
3.) Verwende keine Zeiger auf Zeiger, denn ... warum eigentlich? Ach ja, das spart nämlich sehr viel Speicher und das ist <template Begruendung bitte> auch nicht gut.
Man würde natürlich nie irgendetwas wrappen, damit es leichter verwendbar ist, oder?
4.) Als Elementfunktionen überladene arithmetische Operatoren sind nicht gut. Die sind nämlich zu wenig fehleranfällig, besser du überlädst sie global und lässt dich davon überraschen, was unter Mitwirkung Deiner Konvertierungskonstruktoren oft an interessanten Ergebnissen dabei heraus kommt.
Lies bei zeiten mal ein C++ Buch. Wie etwa Effective C++ oder ähnliches.
(ach und explicit ist zwar alles andere Plattformunabhängig, aber wer braucht das schon bei Klassen für mathematische Objekte? Sind ja sowieso alle Plattformspezifisch).
Ok, du verarscht mich.
Falls du der echte bist (bitte demnächst durch einloggen bestätigen/widerlegen) beende ich die Diskussion mit dir. Denn verarschen lasse ich mich nicht.--- Ein Projekt kann man frontal angehen, also einfach drauflosprogrammieren,
--- wenn einem der Code in einer Funktion zu lang wird, einen Funktionsnamen
--- dafür erfinden, weiterprogrammieren, wieder einen Funktionsnamen erfinden
--- und so weiter und so fort. Irgendwann hat man dann hunderte Funktionen,
--- wahrscheinlich auch eine Lösung für das Problem, und kann sogar von
--- Modularisierung sprechen.--- Oder man kann es aus simplen Bausteinen wie etwa Vektoren, Feldern,
--- Spielsteinen etc., die alle Elemente des Programmes sind, allmählich zu
--- einem Ganzen zusammenfügen. Und letzteres heisst dann objektorientiert.*lol* muss ich das echt kommentieren?
Ich kann bei mir in der Arbeit jeden fragen den ich will, die Antwort wird sein: kleine Funktionen, kleine Module mit kleinen öffentlichen Schnittstellen sind wesentlich wartbarer als ein paar monster funktionen.
Hier in dem Forum sind auch so ziemlich alle meiner Meinung (es gab noch keine Gegenstimme)Denk bei zeiten mal darüber nach.
Natürlich heißt dass es alle so machen nicht, dass es richtig ist. Aber man sollte die Gründe überlegen warum so viele Leute soviel zeit dafür opfern kompakte Funktionen und Module zu entwickeln, wenn es doch einfacher und besser wäre ohne plan alles in eine Funktion zu stecken...
-
MecnelsNichtEingeloggt schrieb:
1.) Verwende keine defines, denn es ist nicht sinnvoll, das gesamte äußere Erscheinungsbild einer Anwendung einfach dadurch verändern zu können, dass man den symbolischen Konstanten andere Werte gibt. Besser ist es, an hundert verschiedenen Positionen irgendwo in hunderten Modulen die tatsächlichen Werte hinzuschreiben, denn wenn man dann dahinter kommt, dass die Textgroesse doch um 2 logische Einheiten größer sein soll, dann hat man das Vergnügen, dutzende Dateien durchsuchen zu müssen und den Wert eben hundertmal ausbessern zu dürfen, und das gleiche nochmal, wenn doch die kleiner Textgröße gereicht hätte. So schlägst du wenigstens Stunden an Zeit tot, was zwar mit define nicht nötig gewesen wäre (da hätte es gereicht den einen Wert daneben auszubessern), aber
das hat keiner gesagt, du sollst "const int GROESSE = 100;" schreiben statt "#DEFINE GROESSE 100". den vorteil solltest du als erfahrener hase doch sofort erkennen.
MecnelsNichtEingeloggt schrieb:
2.) Funktionen sollten ihrem Aufrufer generell keine Auskunft darüber geben, ob während ihrer Ausführung irgend welche Fehler aufgetreten sind, darum sollte man vor allem keine bool Werte zurückgeben, denn dann könnte man ja womöglich den Fehler bei Probeläufen sofort entdecken. Es macht einfach mehr Spaß, in einem großen Programm unter hunderten schweigenden Minifunktionen einen Fehler finden zu müssen.
das hat keiner gesagt, du sollst exceptions verwenden. die sind dafür gemacht.
MecnelsNichtEingeloggt schrieb:
3.) Verwende keine Zeiger auf Zeiger, denn ... warum eigentlich? Ach ja, das spart nämlich sehr viel Speicher und das ist <template Begruendung bitte> auch nicht gut.
das hat keiner gesagt, du sollst std::vector o.ä. container verwenden um schwer auffindbare fehler zu vermeiden.
MecnelsNichtEingeloggt schrieb:
4.) Als Elementfunktionen überladene arithmetische Operatoren sind nicht gut. Die sind nämlich zu wenig fehleranfällig, besser du überlädst sie global und lässt dich davon überraschen, was unter Mitwirkung Deiner Konvertierungskonstruktoren oft an interessanten Ergebnissen dabei heraus kommt. Ist einfach zu lustig (ach und explicit ist zwar alles andere Plattformunabhängig, aber wer braucht das schon bei Klassen für mathematische Objekte? Sind ja sowieso alle Plattformspezifisch).
hä? "ich weiß irgendwas nicht genau, evtl. hatte ich unrecht, also behaupte ich einfach irgendwas" oder wie?