Vorab: Schoen das wir wieder reden ![:slight_smile: :slight_smile:](https://forum.byte-welt.net/images/emoji/twitter/slight_smile.png?v=9)
[quote=mymaksimus]stell dir vor (auch wenn folgendes für ein allgemein schlechtes design sprechen würde…)
du hast ein switch mit 50 cases, jeweils 2 zeilen code unter jedem case + ein break, macht allein 200 zeilen
für diesen switch block… und das kannst du nicht wirklich aufsplitten ^^
edit:
ach… es galt bei dir 10 - 15 zeilen als lang? okay, nimm 5 cases[/quote]
Ja, switch/case, ist ein Konstrukt welches immer zu unschoenen Methoden fuehrt, auch 5 cases haben diese Eigenschaft.
try/catch ist ein anderes Konstrukt was selten sehr kurze Methoden zulaesst.
Gibt bestimmt nochmehr Beispiele, aber: Das ist nicht die Regel, switch/case nimmt man, aber sicherlich nicht sehr haeufig.
[quote=SlaterB;115248]was nebenbei manchmal nicht gut geht wenn mit mehreren lokalen Variablen hantiert wird, es gibt nicht immer nur Simpelcode,
und neue Klassen mit 5 Instanzattributen zu erstellen wäre komplexer als eine Methode mit 5 lokalen Variablen, und mehr Code obendrein
[/quote]
Ich werde jetzt wohl verrueckt wirken, aber: Lokale Variablen blaehen den Code auf!
Deklaration, Belegung/ggf Initialisierung/, Verwendung → Mindestens 3 Zeilen im Regelfall.
Methoden zB. haben den Vorteil dass sie kuerzer sind als lokale Variablen in der Verwendung.
Neue Klassen mit 5 Attributen, zB. als Parameter Obejkt wenn sie keine Logik enthalten, oder als „echte“ Klassen, helfen oft den Code in dem sie verwendet werden kuerzer und klarer zu machen.
[quote=SlaterB;115248]da kann man nicht sinnvoll mal eben an der Stelle mit Variablen listeners, modifiers, mostRecentEventTime, die zweite Hälfte mit der Schleife abtrennen,
eine Methode mit entsprechenden Parametern, die sonst niemand braucht wäre auch (Achtung ) Schwachsinn, einfach nur linear aufgetrennt in zwei Hälften[/quote]
Sehe ich ganz anders.
listeners kommt aus der Methode einer Instanzvariable, braucht man nichtmal ein Paramter fuer.
An mostRecentEventTime kommt man auch nur mit der statischen Methode EventQueue.getMostRecentEventTime().
Hier mal ein sicherlich laecherlicher Versuch das Beispiel zu refakurieren;
protected void fireActionEvent() {
if (!firingActionEvent) {
setFlagToPreventInfiteLoop();
notifyListeners(getModifiers(EventQueue.getCurrentEvent()));
unsetFlagToPreventInfiteLoop();
}
}```
Hab die Implementierungen der extrahierten privaten Methoden weggelassen,, natuerlich ;)
Nebenbei, keine Garantie auf Korrektheit.
IMHO, Dreckiger Code, temp. Variablen werden frueh deklariert und spaet genutzt, Kommentare wie diese "stinken"(einmal die Javadoc zitiert, "schlecht" kommunizierenden Code versucht zu erklaeren), zuviele Einreuckungen (if -> for -> if)
"Extract Method" ist das meistgenutzte Refactoring, muss im Schlaf gehen, ermoeglicht viele Alternativen zur (Um-)Gestaltung bereits geschriebenen Codes, danach werden Dinge wie Cohaesion/Kopplung oft besser erkennbar.
Nebenbei, das Gegenstueck zu extract Method ist inline Method, Refactoring ist eben keine Einbahnstrasse, dasselbe gilt fuer extract Class/Inline Class.
Einfach mal ein bisschen mit dem Code spielen, oft sieht man danach klarer was das Design noch hergeben koennte.
Aber ohne Tests ist ein Spiel mit dem Feuer.
*** Edit ***
Nachtrag:
Ok, hier noch ein Vorschlag, aber keine Garantie, alles nur im Texteditor ;)
protected void fireActionEvent() {
if (!firingActionEvent) {
setFlagToPreventInfiteLoop();
notifyListeners();
unsetFlagToPreventInfiteLoop();
}
}
private void notifyListeners() {
final Object[] listeners = listenerList.getListenerList();
for ( int i = listeners.length-2; i>=0; i-=2 ) {
notifyIfActionListener(listeners, i);
}
}
private void notifyIfActionListener(Object[] listeners, int index) {
if ( isActionListener(listeners[index])) {
// Lazily create the event:
if ( e == null )
e = createActionEvent();
notifyListener(listeners[index+1], );
}
}
private ActionEvent createActionEvent(long eventTime, int modifiers) {
return new ActionEvent(this,
ActionEvent.ACTION_PERFORMED,
getActionCommand(),
EventQueue.getMostRecentEventTime(),
getModifiers(EventQueue.getCurrentEvent()));
}
private boolean isActionListener(Object listener) {
return listener == ActionListener.class;
}
private void notifyListener(Object listener, ActionEvent e) {
listener.actionPerformed(e);
}
private int getModifiers(AWTEvent event) {
if (event instanceof InputEvent)
return ((InputEvent)currentEvent).getModifiers();
if (event instanceof ActionEvent)
return ((ActionEvent)currentEvent).getModifiers();
return 0;
}
49 Zeilen anstatt 30, mit Leerzeilen (6), man koennte noch quetschen aber wozu, vieles sind Namen (Methoden), kein komplexer Code mal abgesehen von der index Arithmetik, kaum temp. Variablen (keine Ahnung wie teuer so ein ActionEvent ist und warum lazy init)..