Best Practice Wann eine Methode schreiben ?

[Mino1337]

Hallo,

ich hab mal eine generelle "Designfrage".
Nun Programmiere ich mein erstes größeres Projekt und bin gerade erst bei der GUI Oberfläche. Doch stellen sich mir jetzt bereits einige Grundsatzfragen.

Wenn ich einen Abschnitt Code mehr als einmal schreiben müsste mit nur geringen abweichungen, ab wann sollte man dafür eine eigene Methode schreiben ?
Oftmals ist es einfacher Copy/Paste und ein, zwei Variablennamen ändern und fertig.
Was gehört da zu einem sauberen Stil ? Gibs einen Knigge für Programmierer ?

Oder was ist wenn ich eine Methode schreibe die zB. 12 integer und ein Objekt eines bestimmten Typs als Parameter benötigt.
Sagen wir ich brauche die selbe Methode nur mit einen anderen Typ von Objekt ?
Ist es "Sauber" diese riesen-Methode zu dublizieren und einfach den einen Parameter zu ändern ?

Ich versuche mir anzugewöhnen Code nicht zu dublizieren als "guten Stil".

Wie seht ihr das ? Erfahrene Programmierer dürfen sich gern auslassen zum Thema Codedesign, ich werde jedes Wort aufsaugen =) ...

Dankeschön ...

 

[Enceladus271]

Mit dem Thema "Guter Programmierstil" kann man ganze Bücher füllen (wurden auch schon, mir ist aber keine freie Online Version bekannt). Insgesamt ist es aber sehr gut doppelten Code zu vermeiden, da hierdurch der Wartungsaufwand und die Fehleranfälligkeit sinkt. Wie genau man das macht kann man nicht im Allgemeinen sagen.

Aber bei deinem obigen Beispiel könnte man sich überlegen statt einer konkreten Klasse ein gemeinsames Interface der zwei verschiedenen Klassen als Parameter zu verwenden. Beispiel:

public void printList(ArrayList list) {
  for (Object o : list) {
    System.out.println(o);
  }
}

public void printSet(HashSet set) {
  for (Object o : set) {
    System.out.println(o);
  }
}

// Beide Methoden kann man zu einer zusammenfassen.
// Diese kann mit allen Arten von Listen und Sets aufgerufen werden (noch noch mehr)

public void printIterable(Iterable iterable) {
  for (Object o : iterable) {
    System.out.println(o);
  }
}

Das ist nur ein stark vereinfachtes Beispiel, aber das Prinzip sollte klar sein: Die Methoden die du brauchst in ein Interface (oder Basisklasse) extrahieren und dann mit dem Interface statt einer konkreten Klasse arbeiten.

 

[Lilianne]

Ah, ich hab das sogar vor kurzem gelesen als ich die Grundlagen nocheinmal durchgin -.- ... Gut nocheinmal daran erinnert zu werden Dankeschön =D ... In der Praxis vergisst man irgendwie die Theorie -.-

 

[Dompteur]

Wiederverwendbarkeit ist nur einer der Gründe, warum man Code-Sequenzen in eine eigene Methode heraushebt.

Ein anderer Grund ist es, lange Methoden zu kürzen. Oft bestehen lange Methode aus einer Folge von Code-Blöcken, die man herauslösen könnte. Vielleicht ist dir auch schon eine Methode untergekommen, wo du einzelne Blöcke hast und diese mit einer Kommentarzeile einleitest. Diese Kommentare sind oft schön griffig und eignen sich (etwas umformuliert) gut als Namen einer neuen Methode.

Grundsätzlich sollte eine Methode eine und nur eine Aufgabe erledigen. Sie sollte einen Namen haben, der klar ausdrückt, welche Aufgabe sie erfüllt.

Mehr zu dem Thema kannst du beispielsweise in Martin Fowlers Klassiker über Refactoring finden.
Hier ein Link dazu : http://refactoring.com/

 

[MrTroble]

Naja ich erinnere hier aber auch an oop. Konstruktoren können ab und zu auch ganz nützlich sein

 

[losgehts]

Hallo,

ich möchte gerne hinzufügen, dass testen auch ein Grund sein kann, auszulagern:

Ich gehöre zwar nicht zu den erfahrenen Programmieren, dennochmöchte ich meine Erfahrung anbringen.
Ich programmiere fast ausschließlich im Umfeld der "angewandten Mathematik". Dabei ist es mir oft wichtig und hilfreich, wenn ich mein Projekt testen kann. Und genau das ist das Problem: testen kann ich nicht ein gesamtes Projekt (zumindest nicht sinnvoll), doch kleine einzelne Teile. Ich habe also viele Methoden wie "calcThis(...), calcThat(...), computeSomething(..). Diese Methoden kann ich einzeln, losgelöst vom Gesamtproblem testen. Und auch wenn ich per "Einzelschritt" debuggen möchte, habe ich bei einem Algorithmus, der so aufgebaut ist:
a = calcA();
b = calcB();
c = calcC();
d = calcD(a, b, c);
Die Möglichkeit Methoden, die ich im Augenglick nicht unter die Lupe nehmen möchte, zwar auszuführen, aber muss sie nicht "im Einzelschritt durchlaufen" (ich weiß, dass ich das selbe Ergebnis auch mit Breakpoints erreichen kann).

Grüße, Ulrich

 

[Mino1337]

@losgehts

Ich bin mir nicht sicher ob ich verstehe was du mir sagen willst, aber andererseits bin ich noch nicht beim debuggen angekommen um einen Schimmer davon zu haben =D. Ich Teste meinen Code immer sobald ich einen neuen Abschnitt fertig gestellt habe. Das Funktioniert, noch. Allerdings wächst mein Projekt schnell und wie gesagt ich habe wenig erfahrung beim Debuggen.

@Domteur

Du sagst eine Methode sollte nur eine Aufgabe erledigen ? Klingt erstmal Logisch, wie weit sollte mand abei gehen ? Ein Beispiel aus meinem Aktuellen Projekt:

private void layerdim(Component a,Container b,GridBagLayout c,int gridx, int gridy, int gridwith, int gridheight,int ipadx,int ipady, double weightx, double weighty, int s, int anchor){
        cons = new GridBagConstraints();     //Bereinigung der Constrains (Alles auf Null setzen weil neu)
        cons.gridx = gridx;                 //In welcher Spalte startet das Objekt
        cons.gridy = gridy;                 //in welcher Zeile endet das Objekt
        cons.gridwidth = gridwith;            //Wieviele Spalten nimmt das Objekt ein
        cons.gridheight = gridheight;         //Wieviele Zeilen nimmt das Objekt ein
        cons.ipadx = ipadx;                 //Wie Breit ist das Objekt Mindestens
        cons.ipady = ipady;                 //Wie Hoch ist das Objekt Mindestens
        cons.weightx = weightx;             //Prozentualer Wert der Breite
        cons.weighty = weighty;             //Prozentualer Wert der Höhe
       
        if(s==0){
            cons.fill = GridBagConstraints.NONE;        //Das Objekt wird nur so gross in seinen Bereich angezeigt wie es ist
        }else if(s==1){
            cons.fill = GridBagConstraints.HORIZONTAL;    //Das Objekt füllt den gesammten Raum Horizontal
        }else if(s==2){
            cons.fill = GridBagConstraints.VERTICAL;    //Das Objekt füllt den gesammten Raum Vertikal
        }else if(s==3){
            cons.fill = GridBagConstraints.BOTH;        //Das Objekt füllt den gesammten Raum Horizontal und Vertikal
        }
       
        if(anchor==0){
            cons.anchor = GridBagConstraints.CENTER;
        }else if(anchor==1){
            cons.anchor = GridBagConstraints.NORTH;
        }else if(anchor==2){
            cons.anchor = GridBagConstraints.EAST;
        }else if(anchor==3){
            cons.anchor = GridBagConstraints.WEST;
        }else if(anchor==4){
            cons.anchor = GridBagConstraints.SOUTH;
        }else if(anchor==5){
            cons.anchor = GridBagConstraints.SOUTHEAST;
        }else if(anchor==6){
            cons.anchor = GridBagConstraints.NORTHEAST;
        }else if(anchor==7){
            cons.anchor = GridBagConstraints.SOUTHWEST;
        }else if(anchor==8){
            cons.anchor = GridBagConstraints.NORTHWEST;
        }
       
        c.setConstraints(a,cons);   
        b.add(a);
    }

Diese Riesenmethode erstellt ein GridBagConstraints Objekt, übergibt es dem übergebenen Layout und addet das übergebene Objekt einem anderen übergebenen Objekt.

Das denke ich meinst du wohl nicht mit "eine Aufgabe" erledigen.
Allerdings, betrachtet man es etwas differenzierter erledigt die Methode die Aufgabe ein Objekt an die richtige stelle eines anderen Objektes zu platzieren.
Ich hab das der übersicht halber getan aber bis dahin hatte mein Code schon 100 andere Formen.

 

[JStein52]

Diese Methode würde für meine Begriffe alles erfüllen was gesagt wurde: sie ist schön klein und übersichtlich (eine Handvoll Anweisungen und zwei Bedingungen auswerten ist ja nicht riesig !) und sie erfüllt eine Aufgabe, nämlich irgendwas zu dimensionieren. (eine Aufgabe erfüllen heisst ja nicht die eine Methode macht die erste Abfrage und eine andere die nächste). Und sie wäre schön einfach zu testen indem man mit dem Debugger hinterher nachschaut wurde alles richtig eingetragen.

 

[Dompteur]

Im Prinzip schaut das recht gut aus.

Ein paar Anmerkungen habe ich aber noch:
Die beiden if-else-Blöcke bilden ja einen Werte-Bereich auf einen anderen ab. Da mapst du deine Konstanten auf jene der GridBagsContraints-Klasse. Das sollte man in eine eigene Methode verlagern. Eventuell brauchst du ja auch ein Mapping in die andere Richtung. Dann könntest du auch die reverse Methode gleich mitimplementieren.
Das wäre auch ein guter Zeitpunkt die Werte 0, 1, 2.. in sprechende Konstanten umzuwandeln.

Ich bin mir nicht sicher, ob die letzten beiden Anweisungen in diese Methode reingehören.
Alles bis auf diese beiden Zeilen dient dazu ein GridBagsContraints Objekt zu erstellen. Die beiden letzten Zeilen stellen eine Beziehung zwischen verschiedenen Objekten her.

Zur Testbarkeit:
Eine Möglichkeit automatisch seine Klassen zu testen, ist der Einsatz von JUnit. Dabei wird zu jeder Klasse eine Testklasse abgeleitet und zu jeder Methode eine oder mehrere Testmethoden geschrieben.
Eine Methode, die ein GridBagsContraints Objekt erstellt, kann da sehr gut getestet werden, indem man im Testcode sicher stellt, dass jede Instanzvariable den erwarteten Wert hat.
JUnit Tests werden dann regelmäßig aufgerufen. Sollte eine deiner Codeänderungen dann dazu führen, dass einer der Tests einen Fehler liefert, weißt du, dass dir entweder ein ungeplanter Seiteneffekt passiert ist oder dein Testfall angepasst werden muss.

 

[losgehts]

Hallo,

ich finde die Methode nicht groß, schon gar nicht zu groß. Ich, derjenige, der nicht weiß, was in der Methode passiert, wundere mich nur über den Methodennamen und die letzten beiden Codezeilen.
Wenn ich die letzten beiden Codezeilen sehe, hätte ich als Name erwartet:
createConstraintsAndsetConstraintsAndAdd (überspitzt formuliert).
Ohne deinen Kontext zu kennen, würde ich dann überlegen, die letzten beiden Zeilen rauszunehmen und zu versuchen, den Methodennamen noch etwas sprechender zu wählen. Aber wie gesagt: Ich gehöre zu den Amateuren unter denjenigen, die sich mit Java / Programmierung beschäftigen.

Mein Vorschlag kommt daher, dass ich mich versuche in denjenigen hineinzuversetzen, der den Code ließt, der die Methode aufruft.
Das würde - wenn man die letzten Zeilen weglässt - ja dann so aussehen:

cons = createConstraint( gridx, gridy, .....)
c.setConstraints(a, cons); 
b.add(a);

Und das finde ich zum Lesen äußerst angenehm.

Das von Dompteur vorgeschlagene JUnit werde ich mir mal für mich anschauen. Für so "standardisierte Tests" habe ich bisher gerne mal die main-Methode meiner Klasse missbraucht; jetzt aber bitte nicht hauen ;-) .

Grüße, Ulrich

 

[Saheeda]

@Mino1337

Warum gibt es die Variablen s und anchor? Warum übergibst du nicht stattdessen direkt die gewünschten Enum-Werte?
Ich würde an der Stelle ansonsten glaube ich eher zu switch-cases tendieren anstelle der if-Kaskaden.

 

[Flown]

Ich muss @Saheeda recht geben. Für was eine neue Konstante einführen, die mir ein 1:1 mapping bietet.

 

[Mino1337]

Ich habe es nicht hinbekommen den direkten Enum-Wert zu übergeben. Da hat Eclipse immer rumgemeckert ... Wie kann man denn soetwas übergeben als String ? Ich hab noch nicht rausgefunden wie diese Constanten heissen so dass Java es versteht, mir fehlt quasi der Oberbegriff =D ...

 

[Flown]

Was heißt dir fehlt der Überbegriff? Hier hast du mal den Link zur DOKU. Da drinnen stehen alle Typen und du siehst, dass alle Konstanten hier ein int sind.

Dann kannst du dein Rumpf so gestalten: layerdim(..., int fill, int anchor, ...) {...} und aufrufen mit layerdim(..., GridBagConstraints.VERTICAL, GridBagConstraints.EAST, ...);

 

[Mino1337]

Es hat eine weile gedauert aber ich bin jetzt hinter die Funktionsweise gestiegen wie die enums Funktionieren =D ... Mich hat immer irritiert dass dort "int" steht man aber eine Methode angibt.
Jetz hab ichs gerafft und meinen Code verschlankt =)