Zyklomatische Komplexität von if-else-... verringern

[Ein ehemaliger Benutzer]

Hallo, manchmal sieht man ja Kleinigkeiten nicht und ich frage mich gerade, ob hier noch weiter vereinfacht werden kann, oder ob die Komplexität nicht weiter verringert werden kann:

        String text1, text2;
        Object o1, o2;
        boolean a, b ;
        if (a) {
            if (b) {
                if (o1 != null) {
                    f1(o1);
                }
                f2();
            } else {
                if (o2 != null) {
                    f3(o2);
                }
                f4();
            }
            f5(text2);
        } else {
            if (b) {
                if (o1 != null) {
                    f6(o1);
                }
            } else {
                if (o2 != null) {
                    f7(o2);
                }
            }
            f5(text1);
        }

Bitte als Pseudocode ansehen und die Methoden f1...f7 dazu denken.

[SeppJ]

Was du da hingeschrieben hast, ist nicht weiter reduzierbar. Ob deine Abstraktion dahinter stimmt, kann man natürlich nicht sagen. Und auch nicht, ob es Sinn ergeben würde, eine andere Abstraktionsebene zu wählen (Vielleicht ist das ein Fall für eine Rules-Engine?); oder ob deine Programmiersprache Konstrukte bietet, die gleiche Logik schöner auszudrücken (In Python würde dies z.B. nach einem Fall für Pattern-Matching aussehen).

[Leon0402]

Da in jedem Fall etwas unterschiedliches gemacht wird, ist das von der Komplexität nicht weiter verringerbar soweit ich das beurteilen kann. Dazu müssten schon Gemeinsamkeiten in den branches vorhanden sein, die durch deine Darstellung ggf. verloren gegangen sind. In der Praxis ist Code der so ähnlich aussieht häufiger mal vereinfachbar.

Interessanter ist an der Stelle dann vlt. eher die Frage, ob man den Code wenigstens schöner schreiben kann.

[Ein ehemaliger Benutzer]

Danke euch. Hier ist mal die gesamte Methode. Es geht um Threads.

    public synchronized boolean next() {
        final int orgIndex = index;
        index = (index + 1) % (array.length + 1);
        new Thread(
                        () -> {
                            try {
                                if (orgIndex < array.length) {
                                    if (aMember) {
                                        if (executor1 != null) {
                                            executor1.shutdown();
                                            //noinspection ResultOfMethodCallIgnored
                                            executor1.awaitTermination(1, TimeUnit.HOURS);
                                        }
                                        executor1 = Executors.newSingleThreadScheduledExecutor();
                                        executor1.scheduleAtFixedRate(array[orgIndex], 0, 10, TimeUnit.SECONDS);
                                    } else {
                                        if (executor2 != null) {
                                            executor2.join();
                                        }
                                        executor2 = new Thread(array[orgIndex]);
                                        executor2.start();
                                    }
                                    button.setText(text2);
                                } else {
                                    if (aMember) {
                                        if (executor1 != null) {
                                            executor1.shutdown();
                                            //noinspection ResultOfMethodCallIgnored
                                            executor1.awaitTermination(1, TimeUnit.HOURS);
                                            executor1 = null;
                                        }
                                    } else {
                                        if (executor2 != null) {
                                            executor2.join();
                                            executor2 = null;
                                        }
                                    }
                                    button.setText(text1);
                                }
                            } catch (InterruptedException ex) {
                                //...
                            }
                        })
                .start();
        return orgIndex < array.length;
    }

Einfach ausgedrückt: Wenn der Benutzer auf eine Schaltfläche klickt, soll alle 10 Sekunden etwas ausgeführt werden, bis er erneut auf diese Schaltfläche klickt.

[hustbaer]

                                if (aMember) {
                                    if (executor1 != null) {
                                        executor1.shutdown();
                                        //noinspection ResultOfMethodCallIgnored
                                        executor1.awaitTermination(1, TimeUnit.HOURS);
                                        executor1 = null;
                                    }
                                    if (orgIndex < array.length) {
                                        executor1 = Executors.newSingleThreadScheduledExecutor();
                                        executor1.scheduleAtFixedRate(array[orgIndex], 0, 10, TimeUnit.SECONDS);
                                    }
                                } else {
                                    if (executor2 != null) {
                                        executor2.join();
                                        executor2 = null;
                                    }
                                    if (orgIndex < array.length) {
                                        executor2 = new Thread(array[orgIndex]);
                                        executor2.start();
                                    }
                                }
                                button.setText(orgIndex < array.length ? text2 : text1);

Vielleicht noch ne Variable für den Ausdruck orgIndex < array.length machen - das kommt ja durchaus öfter vor.

[Ein ehemaliger Benutzer]

Danke!

[Ein ehemaliger Benutzer]

@hustbaer Kannst du bitte noch einmal schauen? Ich blicke da jetzt nicht mehr dran lang, was wie vereinfacht werden könnte. Es lässt sich übersetzen und es gibt keine Fehler, was aber noch nicht heißt, dass es auch gut ist ... Ich fange mal Top-down an:

// edit

[hustbaer]

Sorry, aber Code-Reviews in dem Umfang in Java (was ich nur rudimentär kann) mach ich nicht.

[Ein ehemaliger Benutzer]

@hustbaer sagte in Zyklomatische Komplexität von if-else-... verringern:

Sorry, aber Code-Reviews in dem Umfang in Java (was ich nur rudimentär kann) mach ich nicht.

Ok, kein Problem, dann eben nicht.