Best Practice Code refactern

[lam_tr]

Hallo zusammen,

wie würdet ihr diesen Code schöner schreiben, oft komme ich zu so ein Pattern und weiß halt nicht wie es besser gehen kann. Dieser Code soll Tabellenspalten sortieren, jenachdem welche Spalte selektiert ist. Es geht mir nur rein ums schöner schreiben.

    private void initTableSort() {
        boolean isAsc = buttonASC.isSelected();
        if (radioId.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return o1.getId().compareTo(o2.getId());
                }
                return o2.getId().compareTo(o1.getId());
            });
        } else if (radioBetrag.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return Double.valueOf(o1.getAmount()).compareTo(Double.valueOf(o2.getAmount()));
                }
                return Double.valueOf(o2.getAmount()).compareTo(Double.valueOf(o1.getAmount()));
            });
        } else if (radioDatum.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return o1.getBookDate().compareTo(o2.getBookDate());
                }
                return o2.getBookDate().compareTo(o1.getBookDate());
            });
        } else if (radioKonto.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return o1.getAccount().getName().compareTo(o2.getAccount().getName());
                }
                return o2.getAccount().getName().compareTo(o1.getAccount().getName());
            });
        } else if (radioTyp.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return o1.getBookingType().compareTo(o2.getBookingType());
                }
                return o2.getBookingType().compareTo(o1.getBookingType());
            });
        } else if (radioVorgang.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return o1.getProcess().getName().compareTo(o2.getProcess().getName());
                }
                return o2.getProcess().getName().compareTo(o1.getProcess().getName());
            });
        }else if (radioTaxRate.isSelected()) {
            masterDataService.getBookings().sort((o1, o2) -> {
                if (isAsc) {
                    return (int) (o1.getTaxRate()-o2.getTaxRate());
                }
                return (int) (o2.getTaxRate()-o1.getTaxRate());
            });
        }

        filteredTableView.setInput(masterDataService.getBookings());
    }

Grüße
lam

 

[LimDul]

Grundsätzlich gibt es bestimmt mehre Möglichkeiten. Das erste was mir ins Auge auffällt, ist der "lange" Lambda-Ausdruck. Ich versuche Ausdrücke der Form

(x) -> {
// Code
}

zu vermeiden, weil ich die schlecht lesbar halte und sie einfach enorm viel Platz aufnehmen.

Ich würde daher das erzeugen der Comparatoren und das sortieren trennen.

Erzeugen:

Comparator comparator = null;
if (radioId.isSelected()) {
comparator = Comparator.comparing(Booking:getId);
} else if (radioBetrag.isSelected()) {
comparator = Comparator.comparing(Booking:getAmount);
}
.// usw
if (!isAsc && comparator != null) {
comparator = comparator.reverse();
}
if (cmparator!=null) {
masterDataService.getBookings().sort(comparator);
}

Musst schauen, ob das erzeugen der Comparatoren grundsätzlich so geht (bei Amount bin ich mir nicht sicher, da das ggf. vorher noch in einen Double konvertiert werden muss)

Aber so finde ich das übersichtlicher.

Als weitere Ausbaustufe könnte man sich auch eine enum BookingSortKriterium überlegen und dann eine Methode getSortKriterium die je nach selektiertem RadioButton den entsprechenden Wert zurückliefert. Dann vereinfacht sich die if / else if Cascade in ein reines switch/case.

Als weiteren Schritt könnte man sich dann überlegen, das erzeugen der Comparatoren und das sortieren in eine eigene Klasse/Methode auszulagern, die als Eingabe bekommt:
* Die Liste
* Den Enum mit den Suchkriterium

Und die dann die Liste sortiert. Dann kann man die getrennt testen, die ist dann auch komplett unabhängig von der UI-Logik und im UI läge dann nur das Mapping radrioButtons => Enum und das auslesen/setzen der Liste.

 

[lam_tr]

Grundsätzlich gibt es bestimmt mehre Möglichkeiten. Das erste was mir ins Auge auffällt, ist der "lange" Lambda-Ausdruck. Ich versuche Ausdrücke der Form

(x) -> {
// Code
}

zu vermeiden, weil ich die schlecht lesbar halte und sie einfach enorm viel Platz aufnehmen.

Ich würde daher das erzeugen der Comparatoren und das sortieren trennen.

Erzeugen:

Comparator comparator = null;
if (radioId.isSelected()) {
comparator = Comparator.comparing(Booking:getId);
} else if (radioBetrag.isSelected()) {
comparator = Comparator.comparing(Booking:getAmount);
}
.// usw
if (!isAsc && comparator != null) {
comparator = comparator.reverse();
}
if (cmparator!=null) {
masterDataService.getBookings().sort(comparator);
}

Musst schauen, ob das erzeugen der Comparatoren grundsätzlich so geht (bei Amount bin ich mir nicht sicher, da das ggf. vorher noch in einen Double konvertiert werden muss)

Aber so finde ich das übersichtlicher.

Als weitere Ausbaustufe könnte man sich auch eine enum BookingSortKriterium überlegen und dann eine Methode getSortKriterium die je nach selektiertem RadioButton den entsprechenden Wert zurückliefert. Dann vereinfacht sich die if / else if Cascade in ein reines switch/case.

Als weiteren Schritt könnte man sich dann überlegen, das erzeugen der Comparatoren und das sortieren in eine eigene Klasse/Methode auszulagern, die als Eingabe bekommt:
* Die Liste
* Den Enum mit den Suchkriterium

Und die dann die Liste sortiert. Dann kann man die getrennt testen, die ist dann auch komplett unabhängig von der UI-Logik und im UI läge dann nur das Mapping radrioButtons => Enum und das auslesen/setzen der Liste.

Super klasse, genau sowas habe ich gesucht. Das mit dem Account geht nicht, man muss es zuerst konvertieren, wie? Und der zweite Ansatz mit dem Einpacken der Sortierung in eine eigene Klasse ist auch cool.

 

[lam_tr]

Ist das Konvertieren des Accounts hiermit gemeint

comparator = Comparator.comparing( t -> ((Booking)t).getAccount().getName());

Aber dann ist dieser Cast im Lamda nicht schön oder?

 

[MoxxiManagarm]

Ich kann dem Ansatz von LimDul kaum etwas hinzufügen. Einzig und alleine würde ich die if-elses noch entfernen, zumindest an dieser Stelle. Du hast Radiobuttons, d.h. es gibt keinen Multiselect. Ich würde auf die Änderung reagieren und jedem Radio-Button einen Value zuordnen. Dieser Value nenne ich jetzt einfach mal "selectedComparator". Du speicherst also den Comparator beim Event Radio Button Selection Changed.

Dann ist der Code zum Sortieren relativ straight forward:

Comparator<Booking> selectedComparator; // wird durch die Auswahl eines RadioButtons verändert

if (!buttonASC.isSelected()) {
  selectedComparator = selectedComparator.reversed();
}
masterDataService.getBookings().sort(selectedComparator);

 

[lam_tr]

Ich kann dem Ansatz von LimDul kaum etwas hinzufügen. Einzig und alleine würde ich die if-elses noch entfernen, zumindest an dieser Stelle. Du hast Radiobuttons, d.h. es gibt keinen Multiselect. Ich würde auf die Änderung reagieren und jedem Radio-Button einen Value zuordnen. Dieser Value nenne ich jetzt einfach mal "selectedComparator". Du speicherst also den Comparator beim Event Radio Button Selection Changed.

Dann ist der Code zum Sortieren relativ straight forward:

Comparator<Booking> selectedComparator; // wird durch die Auswahl eines RadioButtons verändert

if (!buttonASC.isSelected()) {
  selectedComparator = selectedComparator.reversed();
}
masterDataService.getBookings().sort(selectedComparator);

Sorry habe das noch nicht ganz verstanden wie du das mit Comparator in RadioButton verpacken meinst. Aber dein Ansatz wäre das entfernen der If/else oder?

 

[MoxxiManagarm]

Ja. Du könntest jedem RadioButton einen ActionListener ergänzen.
Also z.B.

radioId.addActionListener((ActionEvent e) -> selectedComparator = Comparator.comparingInt(Booking::getId));

Dann hast du an keiner Stelle ein riesiges if-else Konstrukt.
Die Sortierfunktion musst du dann nie wieder anfassen, wenn ein Feld hinzukommt. Wenn ein Feld hinzzukommt, welches du für de Sortierung anbietest, dann ergänzt du einfach wieder einen RadioButton der den entsprechenden Comparator als selected setzt.

Also das ActionEvent ist ein Beispiel für Swing. Du verwendest SWT? Da sieht es möglicher Weise etwas anders aus, aber ich denke das Prinzip ist klar.

 

[lam_tr]

Ja. Du könntest jedem RadioButton einen ActionListener ergänzen.
Also z.B.

radioId.addActionListener((ActionEvent e) -> selectedComparator = Booking::getId);

Dann hast du an keiner Stelle ein riesiges if-else Konstrukt.
Die Sortierfunktion musst du dann nie wieder anfassen, wenn ein Feld hinzukommt. Wenn ein Feld hinzzukommt, welches du für de Sortierung anbietest, dann ergänzt du einfach wieder einen RadioButton der den entsprechenden Comparator als selected setzt.

Interesssant, und wo trigger ich die Sortierung? Meine RadioButtons rufen ohnehin schon die #initTableSort() Methode auf

 

[MoxxiManagarm]

Dann könntest du selectedComparator nicht speichern und ihn stattdessen der initTableSort Methode als Parameter übergeben.

Also sinngemäß

radioId.addActionListener((Actionevent e) -> initTableSort(Comparator.comparingInt(Booking::getId)));

private void initTableSort(Comparator<Booking> comparator) {

 

[LimDul]

Interesssant, und wo trigger ich die Sortierung? Meine RadioButtons rufen ohnehin schon die #initTableSort() Methode auf

Dann las sie doch direkt den Comparator in die initTableSort Methode übergeben.

 

[lam_tr]

Dann könntest du selectedComparator nicht speichern und ihn stattdessen der initTableSort Methode als Parameter übergeben.

Also sinngemäß

radioId.addActionListener((Actionevent e) -> initTableSort(Comparator.comparingInt(Booking::getId)));

private void initTableSort(Comparator<Booking> comparator) {

So geht das bei mir leider nicht. Ich habe es so implementiert

    public void onButtonAction(ActionEvent event) {
        Object source = event.getSource();
        if (source == buttonEinnahmenErstellen) {
           dispatchOnEinnahmeErstellen();
        }  else if (source == buttonNextDay) {
           ....
        } else if (source == buttonPreviousDay) {
           ....
        } else if (source == radioBetrag || source == radioDatum || source == radioId || source == radioKonto
                || source == radioTyp || source == radioVorgang || source == buttonASC) {
            initTableSort();
        }
    }

Ich weiß nicht ob das ein guter Ansatz ist, aber zumindest ist das eine zentrale Stelle für mich wenn ich nach Listener schauen will. Es wurde dann gehen wenn ich es ausplitte , dann wäre die if /else zweige hier anstatt in der initTableSort() Methode.

 

[LimDul]

Ich würde lieber jedem Button einen eigenen Listener spendieren (Das ist Java FX, oder? Da bin ich nicht so firm drin).

Grundsätzlich gilt: Keine Scheu vor mehr Klassen. Lieber aus einer langen Methode mehrere kleinere machen und dann aus einer Klasse mit vielen Methoden mehrere Klassen machen.

Bei dem Code oben würde eine statische Codeanalyse wie CheckStyle oder SpotBugs dir auf die Finger klopfen, weil die If-Bedingung viel zu viele Bedingungen hat.

Edit, Nachtrag:
Hier ist die Checkstyle Regel: https://checkstyle.sourceforge.io/a...metrics/BooleanExpressionComplexityCheck.html

Property max - Specify the maximum number of boolean operations allowed in one expression. Default value is 3.

 

[MoxxiManagarm]

Ich weiß nicht ob das ein guter Ansatz ist

Aus meiner Sicht nicht. Ich kann diesen Ansatz nicht leiden Das wird schnell unübersichtlich und fehleranfällig. Aber vor allem unflexibel, wie du nun siehst. Ich mag selbst halt auch keine großen if-else Konstrukte. Ich kann für fast alle if-else Konstrukte andere Varianten finden. Du hast hier halt auch ein solch riesiges Konstrukt.

Außerdem geben if-else einer Methode Komplexität, welche durch Code-Check-Programme durchaus auch angemeckert werden kann. Das wird dann im beruflichen Umfeld relevant.

 

[lam_tr]

Ich würde lieber jedem Button einen eigenen Listener spendieren (Das ist Java FX, oder? Da bin ich nicht so firm drin).

Grundsätzlich gilt: Keine Scheu vor mehr Klassen. Lieber aus einer langen Methode mehrere kleinere machen und dann aus einer Klasse mit vielen Methoden mehrere Klassen machen.

Bei dem Code oben würde eine statische Codeanalyse wie CheckStyle oder SpotBugs dir auf die Finger klopfen, weil die If-Bedingung viel zu viele Bedingungen hat.

Ja genau, ist JavaFX. Schlussendlich mache ich für jeden Control schon eine Methode. die onButtonAction Methode dispatch an der Stelle die controls zu den Listenern, ist das in dem Fall wirklich nicht Sinnvoll (wahrscheinlich wegen de 20 If/else Zweige wa?).

 

[lam_tr]

Ich kann für fast alle if-else Konstrukte andere Varianten finden

Was wäre das? Ich habe auch schon oft gelesen dass man sparsam damit umgehen soll if / else, aber wie geht man sonst anders vor. Wäre ein Switch Case mit Enum wie @LimDul oben erwähnt schon sinnvoller oder?

 

[MoxxiManagarm]

Ein Alternativ-Vorschlag hätte ich noch, wie du bei deinem ActionListener bleiben kannst. Dann müsstest du eine SubKlasse von deinem RadioButton erstellen, also Sinngemäß

class SortingRadioButton extends RadioButton {
  private Comparator<Booking> comparator;

  public SortingRadioButton(Comparator<Booking> comparator) {
    super();
    this.comparator = comparator;
  }
}


... 
else if (source == radioBetrag || source == radioDatum || source == radioId || source == radioKonto
                || source == radioTyp || source == radioVorgang || source == buttonASC) {
            initTableSort(initTableSort(((SortingRadioButton)source).getComparator()));
        }

 

[MoxxiManagarm]

Was wäre das? Ich habe auch schon oft gelesen dass man sparsam damit umgehen soll if / else, aber wie geht man sonst anders vor.

Ein eigener Listener für jeden Control für deinen Fall, außer es ist irgendwo echt sinnvoll den selben Listener zu verwenden.

 

[LimDul]

Was wäre das? Ich habe auch schon oft gelesen dass man sparsam damit umgehen soll if / else, aber wie geht man sonst anders vor. Wäre ein Switch Case mit Enum wie @LimDul oben erwähnt schon sinnvoller oder?

Das ist eine Möglichkeit.

Oft ist aber sinnvoller "weiter oben" (was immer das konkret heißt) schon zu verzweigen. Beispiel mit der onAction Methode:

n Elemente gehen auf eine onAction Methode, die wiederum an m andere Methoden delegiert. Elemeniere den Zwischenschritt das an eine Stelle zu verdichten und verpasse jedem Element eine eigene onAction Methode - da kann man mit Sicherheit auch viel mit Lambda Ausdrücken arbeiten um direkt an die Methoden zu delegieren.

 

[LimDul]

Ein Alternativ-Vorschlag hätte ich noch, wie du bei deinem ActionListener bleiben kannst. Dann müsstest du eine SubKlasse von deinem RadioButton erstellen, also Sinngemäß

class SortingRadioButton extends RadioButton {
  private Comparator<Booking> comparator;

  public SortingRadioButton(Comparator<Booking> comparator) {
    super();
    this.comparator = comparator;
  }
}


...
else if (source == radioBetrag || source == radioDatum || source == radioId || source == radioKonto
                || source == radioTyp || source == radioVorgang || source == buttonASC) {
            initTableSort(initTableSort(((SortingRadioButton)source).getComparator()));
        }

Dann könnte man eigentlich da schreiben

else if (source instanceof SortingRadioButton) { // usw.

 

[lam_tr]

Ein Alternativ-Vorschlag hätte ich noch, wie du bei deinem ActionListener bleiben kannst. Dann müsstest du eine SubKlasse von deinem RadioButton erstellen, also Sinngemäß

class SortingRadioButton extends RadioButton {
  private Comparator<Booking> comparator;

  public SortingRadioButton(Comparator<Booking> comparator) {
    super();
    this.comparator = comparator;
  }
}


...
else if (source == radioBetrag || source == radioDatum || source == radioId || source == radioKonto
                || source == radioTyp || source == radioVorgang || source == buttonASC) {
            initTableSort(initTableSort(((SortingRadioButton)source).getComparator()));
        }

Das würde eventuell gehen wen die Klasse in mein FXML lade. Aber ich glaube ich mache für jeden Control einen eigenen Listener und gebe dort die Sortierung mit, wie du weiter oben schon erwähnt hast.

 

[tommysenf]

private void initTableSort() {
        boolean isAsc = buttonASC.isSelected();
        
        masterDataService.getBookings().sort((o1, o2) -> {
                 int comparism = 0;
                 if (radioId.isSelected()) {
                        comparism = o1.getId().compareTo(o2.getId());
                 } else if(radioBetrag.isSelected) {
                       comparism = Double.valueOf(o1.getAmount()).compareTo(Double.valueOf(o2.getAmount()));
                } ....
                return isAsc ? comparism : comparism *= -1
            });
        filteredTableView.setInput(masterDataService.getBookings());
    }

 

[mrBrown]

Du könntest die RadioButtons in einer ToggleGroup wrappen und den Comparator den RadioButtons jeweils als UserData mitgeben.

Dann bräuchte es nur einen Listener an der ToogleGroup, und den aktuellen Comparator bekämest du über ToogleGroup#getToogle#getUserData.

 

[LimDul]

Vielleicht noch mal eine etwas allgemeinere Anmerkung zu Refaktoring Vorgehen. Was meines Erachtens an der Stelle viel hilft sind Unit-Tests (bis hin zu Testdriven Development). Denn wenn man anfängt Unit-Tests zu schreiben, stellt man schnell fest:

* Ich komme an den Teil schlecht ran, den ich testen will
* Ich brauche riesig Setup-Aufwand um die Konstellation aufzubauen

Dann zerhackt man den Code in kleinere Einheiten (=mehr Methoden/mehr Klassen).

Beispiel auf deinen konkreten Code bezogen, was will man testen?

* Wenn ich Betrag aufsteigend sortieren will, wird korrekt sortiert. Was brauche ich da bei deinem Ursprungscode?
** Ich muss alle Radiobuttons erzeugen
** Ich muss den korrekten Radiobutton auf selected setzen
** Ich brauche einen masterDataService
** Ich brauche eine TableView

So, das ist extrem viel Aufwand - nur um zu testen, ob die Sortierung klappt.

Zerhackt man das in mehrere Teilprobleme:

* Ich ermittle aus den Radio-Buttons den Comparator (kann ich separat testen)
* Ich kann die Comparatoren separat testen
* Ich kann dann testen, ob die Übertragung aus dem masterService in die Sortiermethode klappt
* Ich kann dann testen, dass die Rückgabe aus der Sortiermethode in die TableView klappt

Insgesamt habe ich da über alle Tests sogar weniger Setup-Aufwand als bei dem Test deiner Methode.

Man kommt bei dem Vorgehen oft zu Stellen, wo man - nur um einen Aspekt separat zu testen - entscheidet Teile in eigene Methoden auszulagern.

 

[lam_tr]

Du könntest die RadioButtons in einer ToggleGroup wrappen und den Comparator den RadioButtons jeweils als UserData mitgeben.

Dann bräuchte es nur einen Listener an der ToogleGroup, und den aktuellen Comparator bekämest du über ToogleGroup#getToogle#getUserData.

Stimmt, das ist richtig einfach

 

[lam_tr]

Vielleicht noch mal eine etwas allgemeinere Anmerkung zu Refaktoring Vorgehen. Was meines Erachtens an der Stelle viel hilft sind Unit-Tests (bis hin zu Testdriven Development). Denn wenn man anfängt Unit-Tests zu schreiben, stellt man schnell fest:

* Ich komme an den Teil schlecht ran, den ich testen will
* Ich brauche riesig Setup-Aufwand um die Konstellation aufzubauen

Dann zerhackt man den Code in kleinere Einheiten (=mehr Methoden/mehr Klassen).

Beispiel auf deinen konkreten Code bezogen, was will man testen?

* Wenn ich Betrag aufsteigend sortieren will, wird korrekt sortiert. Was brauche ich da bei deinem Ursprungscode?
** Ich muss alle Radiobuttons erzeugen
** Ich muss den korrekten Radiobutton auf selected setzen
** Ich brauche einen masterDataService
** Ich brauche eine TableView

So, das ist extrem viel Aufwand - nur um zu testen, ob die Sortierung klappt.

Zerhackt man das in mehrere Teilprobleme:

* Ich ermittle aus den Radio-Buttons den Comparator (kann ich separat testen)
* Ich kann die Comparatoren separat testen
* Ich kann dann testen, ob die Übertragung aus dem masterService in die Sortiermethode klappt
* Ich kann dann testen, dass die Rückgabe aus der Sortiermethode in die TableView klappt

Insgesamt habe ich da über alle Tests sogar weniger Setup-Aufwand als bei dem Test deiner Methode.

Man kommt bei dem Vorgehen oft zu Stellen, wo man - nur um einen Aspekt separat zu testen - entscheidet Teile in eigene Methoden auszulagern.

Ja ich weiß was du meinst, ich versuch mal so langsam vor zu gehen, danke!

 

[MoxxiManagarm]

Aber ich glaube ich mache für jeden Control einen eigenen Listener und gebe dort die Sortierung mit, wie du weiter oben schon erwähnt hast.

Wie auch immer du dich letztlich entscheidest, der Grundgedanke läuft in jedem Ansatz darauf hinaus deiner initTableSort Methode mit Hilfe eines Comparators nur noch zu sagen was sie sortieren soll. Du definierst hier immer nur den Ascending Comparator, welcher durch comparator.reversed() bei Bedarf (Descending) umgedreht werden kann. Dadurch nimmt die Komplexität an der Stelle deutlich ab.

 

[MoxxiManagarm]

Du könntest die RadioButtons in einer ToggleGroup wrappen und den Comparator den RadioButtons jeweils als UserData mitgeben.

Sowas meinte ich mit meinem ersten Post mit value. Ich wusste nur nicht wie das bei FX aussieht ^^

 

[LimDul]

Ja ich weiß was du meinst, ich versuch mal so langsam vor zu gehen, danke!

Das ist auch ein Lernprozess, der nie aufhört. Ich hab mittlerweile auch über 10 Jahre Berufserfahrung auf dem Buckel - und just diese Woche eine Review Anmerkung von einem Kollegen bekommen. Da ging es um einen Dialog, wo ich nur einen Test für den "Cancel" Button hatte. Da kam die Anmerkung "Da fehlt ein Test für den OK-Button". Ja, den hatte ich versucht zu schreiben, aber ging halt nicht. Aber dann halt noch mal rangesetzt und aus der Methode, die den Dialog baut zwei gemacht - eine public, eine protected und die Protected so geschnitten von den Parametern, dass ich die Konstellation sinnvoll testen konnte.

 

[lam_tr]

Das ist auch ein Lernprozess, der nie aufhört. Ich hab mittlerweile auch über 10 Jahre Berufserfahrung auf dem Buckel - und just diese Woche eine Review Anmerkung von einem Kollegen bekommen. Da ging es um einen Dialog, wo ich nur einen Test für den "Cancel" Button hatte. Da kam die Anmerkung "Da fehlt ein Test für den OK-Button". Ja, den hatte ich versucht zu schreiben, aber ging halt nicht. Aber dann halt noch mal rangesetzt und aus der Methode, die den Dialog baut zwei gemacht - eine public, eine protected und die Protected so geschnitten von den Parametern, dass ich die Konstellation sinnvoll testen konnte.

Ja das sind die Erfahrungen die man mit sich nimmt. Es ist echt wertvoll wenn man hier auf dem guten Wege geleitet wird, solche Dinge kann man nur gut verstehen wenn man das veranschaulicht bekommt.

 

[lam_tr]

Du könntest die RadioButtons in einer ToggleGroup wrappen und den Comparator den RadioButtons jeweils als UserData mitgeben.

Dann bräuchte es nur einen Listener an der ToogleGroup, und den aktuellen Comparator bekämest du über ToogleGroup#getToogle#getUserData.

Das gefällt mir sehr.

ToggleGroup tg = new ToggleGroup();
        tg.selectedToggleProperty().addListener(new ChangeListener<Toggle>() {
            @Override
            public void changed(ObservableValue<? extends Toggle> observable, Toggle oldValue, Toggle newValue) {
                if (newValue!=null) {
                    Object userData = newValue.getUserData();
                    System.out.println(userData);
                }
            }
        });
        
        radioId.setToggleGroup(tg);
        radioId.setUserData("ID");
        radioKonto.setToggleGroup(tg);
        radioKonto.setUserData("Konto");
        radioBetrag.setToggleGroup(tg);
        radioBetrag.setUserData("Betrag");
        radioDatum.setToggleGroup(tg);
        radioDatum.setUserData("Datum");

Ich habe nur auf die Schnelle getestet, da sollen die Comparatoren rein

@MoxxiManagarm
Jetzt ist es mir wesentlich klarer, danke dir auch.

 

[MoxxiManagarm]

Ich habe als Neuling übrigens auch meistens globale Listener verwendet, allerdings waren das auch alles mini-Projekte ohne Komplexität und noch vor Java 8 Lambdas. Besonders die Lambdas haben viel vereinfacht an der Stelle. Mit steigender Erfahrung weiß man bestimmte Dinge halt zu schätzen und würde es nie wieder anders machen wollen. Natürlich muss man diesen Dingen erstmal begegnen.