Warum ist es eine schlechte Praxis, einen Eventhandler aus dem Code heraus aufzurufen?

73

Angenommen, Sie haben einen Menüpunkt und eine Schaltfläche, die dieselbe Aufgabe ausführen. Warum ist es nicht ratsam, den Code für die Aufgabe in das Aktionsereignis eines Steuerelements einzufügen und dieses Ereignis dann vom anderen Steuerelement aus aufzurufen? Delphi erlaubt dies ebenso wie vb6, realbasic jedoch nicht und sagt, dass Sie den Code in eine Methode einfügen sollten, die dann sowohl vom Menü als auch von der Schaltfläche aufgerufen wird

jjb
quelle
6
Upvoted, da ich glaube, dass jeder, der sich für Delphi-Programmierung interessiert, sich dessen bewusst sein sollte, dass dies eine schlechte Praxis ist. Bevor ich anfing, Aktionen zu verwenden (wie von Rob Kennedy in Punkt 3 erwähnt), hatte ich cooked upeinige spaghettiAnwendungen, die ein wahrer Albtraum sind, und das ist schade, da die Apps sehr nett waren. Aber ich hasste meine eigene Schöpfung. Robs Antwort ist wirklich nett und erschöpfend, IMO.
Peter Perháč

Antworten:

84

Es ist eine Frage, wie Ihr Programm organisiert ist. In dem von Ihnen beschriebenen Szenario wird das Verhalten des Menüelements anhand der folgenden Schaltflächen definiert:

procedure TJbForm.MenuItem1Click(Sender: TObject);
begin
  // Three different ways to write this, with subtly different
  // ways to interpret it:

  Button1Click(Sender);
  // 1. "Call some other function. The name suggests it's the
  //    function that also handles button clicks."

  Button1.OnClick(Sender);
  // 2. "Call whatever method we call when the button gets clicked."
  //    (And hope the property isn't nil!)

  Button1.Click;
  // 3. "Pretend the button was clicked."
end;

Jede dieser drei Implementierungen funktioniert, aber warum sollte der Menüpunkt so abhängig von der Schaltfläche sein? Was ist das Besondere an der Schaltfläche, dass sie den Menüpunkt definieren soll? Was würde mit dem Menü passieren, wenn durch ein neues UI-Design die Schaltflächen wegfallen würden? Eine bessere Möglichkeit besteht darin, die Aktionen des Ereignishandlers herauszufiltern, damit er unabhängig von den Steuerelementen ist, mit denen er verknüpft ist. Dafür gibt es einige Möglichkeiten:

  1. Eine besteht darin, die MenuItem1ClickMethode vollständig zu entfernen und die Button1ClickMethode der MenuItem1.OnClickEreigniseigenschaft zuzuweisen . Es ist verwirrend, Methoden für Schaltflächen zu haben, die den Ereignissen von Menüelementen zugewiesen sind. Daher sollten Sie den Ereignishandler umbenennen. Dies ist jedoch in Ordnung, da die Methodennamen von Delphi im Gegensatz zu VB nicht definieren, welche Ereignisse behandelt werden. Sie können jedem Ereignishandler eine beliebige Methode zuweisen, solange die Signaturen übereinstimmen. Die OnClickEreignisse beider Komponenten sind vom Typ TNotifyEvent, sodass sie eine einzige Implementierung gemeinsam nutzen können. Nennen Sie Methoden für das, was sie tun, nicht für das, wozu sie gehören.

  2. Eine andere Möglichkeit besteht darin, den Ereignishandlercode der Schaltfläche in eine separate Methode zu verschieben und diese Methode dann von den Ereignishandlern beider Komponenten aufzurufen:

    procedure HandleClick;
    begin
      // Do something.
    end;
    
    procedure TJbForm.Button1Click(Sender: TObject);
    begin
      HandleClick;
    end;
    
    procedure TJbForm.MenuItem1Click(Sender: TObject);
    begin
      HandleClick;
    end;
    

    Auf diese Weise ist der Code, der wirklich Dinge erledigt, nicht direkt an eine der Komponenten gebunden, und Sie haben die Freiheit, diese Steuerelemente einfacher zu ändern , z. B. indem Sie sie umbenennen oder durch andere Steuerelemente ersetzen. Das Trennen des Codes von der Komponente führt uns zum dritten Weg:

  3. Die TActionin Delphi 4 eingeführte Komponente wurde speziell für die von Ihnen beschriebene Situation entwickelt, in der mehrere UI-Pfade zu demselben Befehl vorhanden sind. (Weitere Sprachen und Entwicklungsumgebungen bieten ähnliche Konzepte, es zu Delphi nicht eindeutig zuzuordnen ist.) Setzen Sie Ihren Event-Handling - Code in den TAction‚s OnExecuteEvent - Handler, und weisen Sie dann die Aktion an die ActionEigenschaft sowohl der Taste und den Menüpunktes.

    procedure TJbForm.Action1Click(Sender: TObject);
    begin
      // Do something
      // (Depending on how closely this event's behavior is tied to
      // manipulating the rest of the UI controls, it might make
      // sense to keep the HandleClick function I mentioned above.)
    end;
    

    Möchten Sie ein weiteres UI-Element hinzufügen, das sich wie die Schaltfläche verhält? Kein Problem. Fügen Sie es hinzu, legen Sie seine ActionEigenschaft fest und Sie sind fertig. Sie müssen keinen weiteren Code schreiben, damit das neue Steuerelement wie das alte aussieht und sich so verhält. Sie haben diesen Code bereits einmal geschrieben.

    TActiongeht über nur Event-Handler hinaus. Damit können Sie sicherstellen, dass Ihre UI-Steuerelemente einheitliche Eigenschafteneinstellungen haben , einschließlich Beschriftungen, Hinweisen, Sichtbarkeit, Aktiviertheit und Symbolen. Wenn ein Befehl zu diesem Zeitpunkt nicht gültig ist, legen Sie die EnabledEigenschaft der Aktion entsprechend fest. Alle verknüpften Steuerelemente werden automatisch deaktiviert. Sie müssen sich keine Sorgen machen, dass ein Befehl über die Symbolleiste deaktiviert, aber dennoch über das Menü aktiviert wird. Sie können das OnUpdateEreignis der Aktion sogar verwenden, damit sich die Aktion basierend auf den aktuellen Bedingungen selbst aktualisieren kann, anstatt zu wissen, wann etwas passiert, bei dem Sie die EnabledEigenschaft möglicherweise sofort festlegen müssen .

Rob Kennedy
quelle
1
Tolle Antwort, danke. Ich bin besonders beeindruckt von dem TAction-Ansatz, den ich vorher nicht gekannt hatte, der sich aber am besten anhört. Tatsächlich scheint Delphi diesen Bereich gut abgedeckt zu haben und alle Ansätze zuzulassen. Übrigens Sie erwähnen, dass TAction das automatische Deaktivieren der zugehörigen Steuerelemente ermöglicht. Eine Änderung der Einstellung zum Stil in letzter Zeit, die mir gefällt, ist der Trend, Steuerelemente nicht zu deaktivieren, wenn eine Aktion nicht verfügbar ist, sondern dem Benutzer zu erlauben, auf das Steuerelement zu klicken und ihm dann eine Nachricht zu geben, die erklärt, warum die Aktion nicht ausgeführt wird.
JJB
Ein Teil des Vorteils des TAction-Ansatzes gegenüber den anderen Möglichkeiten wird irrelevant, wenn dieser Stil verwendet wird, denke ich.
JJB
3
@jjb: Wenn Steuerelemente nicht deaktiviert werden, obwohl ihre Aktionen nicht verfügbar sind, führt der Geldautomat meiner Meinung nach zu einer sehr verwirrenden Benutzeroberfläche. Da deaktivierte Steuerelemente die Benutzeroberfläche in der Tat weniger erkennbar machen, sollte es einen Hinweis auf die Ursache geben, z. B. QuickInfos oder Hilfemeldungen in der Statusleiste, wenn sich die Maus über einem deaktivierten Steuerelement befindet. Ich bevorzuge diesen Ansatz gegenüber einer Benutzeroberfläche, die keinen Hinweis auf den Zustand gibt, in dem er sich befindet.
mghie
@mghie ja das klingt nach dem besten aus beiden Welten.
JJB
1
<seufz>. Was Sie mit der TAction machen, ist nicht der Punkt. Der Punkt ist, dass Sie damit sicherstellen können, dass alles auf die gleiche Weise funktioniert.
Rob Kennedy
15

Weil Sie die interne Logik von einer anderen Funktion trennen und diese Funktion aufrufen sollten ...

  1. von beiden Event-Handlern
  2. getrennt vom Code, wenn Sie müssen

Dies ist eine elegantere Lösung und viel einfacher zu warten.

rauchen1
quelle
IMO ist dies keine Antwort auf die Frage. Ich fragte, warum Sie nicht A statt B tun können und diese Antwort sagt nur, weil B besser ist!
JJB
Übrigens meine ich nicht, dass es in einem unhöflichen Sinne nur meine Beobachtung ist. Ich denke, Gerald hat mit seiner Antwort den Nagel auf den Kopf getroffen
jjb
2
Die Antwort, dass B eine elegantere Lösung ist und leichter zu pflegen ist, stammt aus meiner persönlichen Erfahrung. Die persönliche Erfahrung ist in der Tat kein Gedanke, den Sie mit harten Daten beweisen können. Dies ist der Unterschied zwischen dem Erleben von etwas und dem wissenschaftlichen Nachweis. Und wenn man über Eleganz spricht ... man kann es nicht definieren, man kann es nur fühlen ... Beziehen Sie sich schließlich auf "Code Complete" von Steve McConnell, er hat eine ziemlich gute Berichterstattung über solche Probleme.
Smok1
Fairer Punkt, aber ich würde sagen, dass die Verwendung persönlicher Erfahrung als Argument Beispiele erfordert, wenn es darum geht, Gewicht zu haben.
JJB
Ok, ich werde meine Codearchive durchsuchen und Code als Beispiel einfügen.
Smok1
10

Dies ist eine Erweiterungsantwort, wie versprochen. Im Jahr 2000 haben wir begonnen, eine Anwendung mit Delphi zu schreiben. Dies war eine EXE-Datei und einige DLLs, die Logik enthielten. Dies war Filmindustrie, also gab es Kunden-DLL, Buchungs-DLL, Kassen-DLL und Abrechnungs-DLL. Wenn der Benutzer die Abrechnung durchführen wollte, öffnete er das entsprechende Formular, wählte den Kunden aus einer Liste aus, lud die OnSelectItem-Logik die Kunden-Theater in das nächste Kombinationsfeld und füllte nach Auswahl des nächsten OnSelectItem-Ereignisses das dritte Kombinationsfeld mit Informationen zu den Filmen, die noch nicht vorhanden waren noch in Rechnung gestellt. Der letzte Teil des Prozesses war das Drücken der Schaltfläche "Rechnung machen". Alles wurde als Event-Prozedur durchgeführt.

Dann entschied jemand, dass wir eine umfassende Tastaturunterstützung haben sollten. Wir haben aufrufende Ereignishandler von anderen geraden Handlern hinzugefügt. Der Workflow von Ereignishandlern begann sich zu verkomplizieren.

Nach zwei Jahren entschied sich jemand, eine andere Funktion zu implementieren, sodass dem Benutzer, der mit Kundendaten in einem anderen Modul (Kundenmodul) arbeitet, die Schaltfläche "Diesen Kunden in Rechnung stellen" angezeigt werden sollte. Diese Schaltfläche sollte das Rechnungsformular auslösen und in einem solchen Zustand anzeigen, als wäre es der Benutzer, der alle Daten manuell ausgewählt hat (der Benutzer sollte in der Lage sein, die Daten anzuzeigen, einige Anpassungen vorzunehmen und die magische Schaltfläche „Rechnung erstellen“ zu drücken ). Da es sich bei den Kundendaten um eine DLL und bei der Abrechnung um eine andere handelte, wurden EXE-Nachrichten an EXE weitergeleitet. Die offensichtliche Idee war also, dass der Kundendatenentwickler eine einzige Routine mit einer einzigen ID als Parameter haben wird und dass sich all diese Logik innerhalb des Abrechnungsmoduls befindet.
Stellen Sie sich vor, was passiert ist. Da sich ALLE Logik in Ereignishandlern befand, haben wir viel Zeit damit verbracht, keine Logik zu implementieren, sondern Benutzeraktivitäten nachzuahmen - wie das Auswählen von Elementen, das Anhalten von Application.MessageBox in Ereignishandlern mithilfe von GLOBAL-Variablen usw. Stellen Sie sich vor, wenn wir sogar einfache logische Prozeduren hätten, die in Ereignishandlern aufgerufen werden, hätten wir die boolesche Variable DoShowMessageBoxInsideProc in die Prozedursignatur einführen können. Eine solche Prozedur könnte mit true-Parametern aufgerufen worden sein, wenn sie vom Ereignishandler aufgerufen wurde, und mit FALSE-Parametern, wenn sie von einem externen Ort aufgerufen wurde.

Das hat mich also gelehrt, Logik nicht direkt in GUI-Ereignishandler zu integrieren, mit einer möglichen Ausnahme von kleinen Projekten.

rauchen1
quelle
1
Danke, dass du das gemacht hast. Ich denke, es zeigt deutlich den Punkt, den Sie gemacht haben. Ich mag die Idee, dass der boolesche Parameter ein anderes Verhalten zulässt, wenn das Ereignis tatsächlich stattgefunden hat, als dass es über Code erfolgt.
JJB
1
Anderes
@jjb: Ich denke, dies ist ein noch umfassenderes Thema, wenn man eine ähnliche Logik in zwei verschiedenen Prozeduren hat. In einer solchen Situation ist es immer besser, die dritte Prozedur mit der tatsächlichen Logik zu versehen und diese beiden ähnlichen Prozeduren in Wrapper für neue Logik mit proc umzuwandeln. Die Unterschiede im Verhalten können durch Steuerparameter vorgenommen werden. Viele Komponenten mit zwei oder mehr Überladungsmethoden wie Open. Diese offenen Methoden sind normalerweise Wrapper für eine private InternalOpen-Prozedur mit booleschen Parametern für einige kleine Anpassungen.
Smok1
@inzKulozik: Ja, Steuerlogik unter Verwendung der UI-Logik und tatsächlich unter Verwendung von Niled Sender als boolesche Steuervariable ... Ich denke, es ist sogar besser, als var a, b, c, d, e, f, g: Ganzzahl nur in zu deklarieren case;)
smok1
8

Trennung von Bedenken. Ein privates Ereignis für eine Klasse sollte in dieser Klasse gekapselt und nicht von externen Klassen aufgerufen werden. Dies erleichtert es Ihrem Projekt, sich später zu ändern, wenn Sie starke Schnittstellen zwischen Objekten haben und das Auftreten mehrerer Einstiegspunkte minimieren.

Chris Ballance
quelle
1
Ich bin mit der Kapselung und Trennung einverstanden, aber Click / Dbclick-Ereignisse auf vb6-Steuerelementen sind niemals privat. Und wenn sie nicht privat gemacht würden, wäre jemand der Meinung, dass der Schaden minimal wäre.
jpinto3912
Weder in Delphi / Lazarus werden sie veröffentlicht (RTTI'd)
Marco van de Voort
@ jpinto3912 - Tatsächlich sind VB6-Ereignishandler standardmäßig privat.
MarkJ
Dies ist kein Ereignis, es ist eine Ereignissenke. Und nicht einmal die Senke selbst, sondern die vom Compiler aufgerufene Logik erzeugte die Senke. Nach dem größten Teil der in diesem Thread gezeigten Logik würde ein VB6-Ereignishandler niemals einen Code enthalten, außer einem Aufruf einer weiteren (neu redundanten) Prozedur! Ehrlich gesagt kaufe ich es nicht und Vorkommen sollten sowieso selten genug sein. Wenn man paranoid ist, kann der Handler, der die Logik implementiert, mit denjenigen gruppiert werden, die sie aufrufen, und Kommentare ausarbeiten, die als Leitfaden für zukünftige Betreuer dienen.
Bob
@ jpinto3912: Ereignisse sind öffentlich, aber Handler sind privat. Ereignisse sind tatsächlich Methoden auf einer (versteckten, aber öffentlichen) Ereignissenkenschnittstelle. Die (privaten) Ereignishandlermethoden sind Implementierungen von Methoden auf der (öffentlichen) Ereignissenkenschnittstelle. Ähnlich wie beim Implementieren einer Schnittstelle mit dem ImplementsSchlüsselwort Privatestandardmäßig Methoden für die Implementierung erstellt werden, mit der Ausnahme, dass Ereignisse und Ereignishandler speziell behandelt werden (dh Sie müssen keine Handler für alle Ereignisse implementieren, die von einer Klasse verfügbar gemacht werden, fügt der Compiler ein leeres Ereignis ein Handler zur Kompilierungszeit).
Mike Spross
8

Angenommen, Sie entscheiden irgendwann, dass der Menüpunkt keinen Sinn mehr ergibt, und möchten den Menüpunkt entfernen. Wenn Sie nur ein anderes Steuerelement haben, das auf die Ereignisbehandlungsroutine des Menüelements verweist, ist dies möglicherweise kein großes Problem. Sie können den Code einfach in die Ereignisbehandlungsroutine der Schaltfläche kopieren. Wenn Sie jedoch verschiedene Möglichkeiten haben, den Code aufzurufen, müssen Sie viele Änderungen vornehmen.

Persönlich mag ich die Art und Weise, wie Qt damit umgeht. Es gibt eine QAction-Klasse mit einem eigenen Ereignishandler, der verknüpft werden kann. Anschließend wird die QAction allen UI-Elementen zugeordnet, die diese Aufgabe ausführen müssen.

Gerald
quelle
1
OK, das ist logisch für mich. Wenn Sie die Schaltfläche löschen, haben Sie nichts zu sagen, dass andere Steuerelemente darauf verweisen. Gibt es noch andere Gründe?
JJB
3
Delphi kann das Gleiche tun. Weisen Sie dem Menü und der Schaltfläche eine Aktion zu. Dies mache ich ständig für Symbolleistenschaltflächen, die die Menüfunktionalität widerspiegeln.
TheArtTrooper
1
Ein weiterer Grund ist, dass Sie möglicherweise eine Art Benutzeroberflächenaktualisierung durchführen möchten, wenn ein Menüelement ausgewählt wird, das bei Auswahl der Schaltfläche nicht gilt. In den meisten Fällen ist es an sich nichts Schlechtes, das zu tun, was Sie sagen, aber es ist nur eine fragwürdige Entwurfsentscheidung, die die Flexibilität einschränkt.
Gerald
8

Ein weiterer wichtiger Grund ist die Testbarkeit. Wenn Ereignisbehandlungscode in der Benutzeroberfläche vergraben ist, können Sie dies nur durch manuelles Testen oder automatisiertes Testen testen, das stark an die Benutzeroberfläche gebunden ist. (zB Menü A öffnen, Schaltfläche B anklicken). Jede Änderung der Benutzeroberfläche kann dann natürlich Dutzende von Tests unterbrechen.

Wenn der Code in ein Modul umgestaltet wird, das sich ausschließlich mit dem Job befasst, den er ausführen muss, wird das Testen viel einfacher.

Colin Goudie
quelle
4

Es ist offensichtlich ordentlicher. Aber auch Benutzerfreundlichkeit und Produktivität sind natürlich immer wichtig.

In Delphi verzichte ich in seriösen Apps generell darauf, aber ich rufe Eventhandler in kleinen Dingen an. Wenn sich kleine Dinge irgendwie in etwas Größeres verwandeln, räume ich es auf und erhöhe normalerweise gleichzeitig die Trennung zwischen Logik und Benutzeroberfläche.

Ich weiß jedoch, dass es in Lazarus / Delphi keine Rolle spielt. Andere Sprachen weisen möglicherweise ein spezielleres Verhalten bei Eventhandlern auf.

Marco van de Voort
quelle
Klingt nach einer pragmatischen Politik
jjb
2

Warum ist es schlechte Praxis? Weil es viel einfacher ist, Code wiederzuverwenden, wenn er nicht in UI-Steuerelemente eingebettet ist.

Warum kannst du das nicht in REALbasic machen? Ich bezweifle, dass es einen technischen Grund gibt. Es ist wahrscheinlich nur eine Designentscheidung, die sie getroffen haben. Es erzwingt sicherlich bessere Codierungspraktiken.

Paul Lefebvre
quelle
Ist das ein Argument dafür, nichts außer Aufrufen von Ereignissen zuzulassen? Das Suchen nach Code würde immer etwas länger dauern, wenn Sie zuerst im Ereignis nach dem Namen der Methode suchen müssen, in der sich der Code befindet. Außerdem wird es sehr mühsam, sich sinnvolle Namen für endlos viele Methoden ausdenken zu müssen.
JJB
1
Nein, es ist ein Argument dafür, dass nicht versucht wird, Code in Ereignissen wiederzuverwenden. Wenn der Code nur für das Ereignis gilt, würde ich ihn in das Ereignis einfügen. Aber wenn ich es von irgendwo anders aufrufen muss, überarbeite ich es in eine eigene Methode.
Paul Lefebvre
Ja, dieser Ansatz scheint sehr sinnvoll zu sein. Danke
jjb
1

Angenommen, Sie haben irgendwann entschieden, dass das Menü etwas anders aussehen soll. Vielleicht geschieht diese neue Änderung nur unter bestimmten Umständen. Sie vergessen die Schaltfläche, aber jetzt haben Sie auch ihr Verhalten geändert.

Wenn Sie dagegen eine Funktion aufrufen, ist es weniger wahrscheinlich, dass Sie ihre Funktionsweise ändern, da Sie (oder der nächste) wissen, dass dies schlimme Folgen hat.

Tomjen
quelle
1
Ich bin mit Ihrer Logik nicht einverstanden. Wenn Sie einen Menüpunkt und eine Schaltfläche haben, um dasselbe zu tun, sollten sie dasselbe tun und nicht anders funktionieren. IOW: Wenn Sie über ein Menüelement verfügen, mit dem Sie die aktuelle Zeile in einer Datenbank bearbeiten können, und über eine Schaltfläche, mit der Sie die aktuelle Zeile in einer Datenbank bearbeiten können, sollten beide dasselbe tun. Wenn nicht, sollten sie nicht beide "Bearbeiten" heißen.
Ken White
@ Ken Es kann gute Gründe für das Menü und die Schaltfläche geben, verschiedene Dinge zu tun. Wenn der Benutzer beispielsweise in VB6 auf einen Menüpunkt klickt, wird kein Ereignis mit verlorenem Fokus auf dem Steuerelement mit dem Fokus ausgelöst. Wenn der Benutzer auf eine Schaltfläche klickt, werden Ereignisse mit verlorenem Fokus ausgelöst. Wenn Sie sich auf Ereignisse mit verlorenem Fokus verlassen (z. B. um eine Validierung durchzuführen), benötigen Sie möglicherweise speziellen Code im Menü-Klick-Ereignis, um einen Fokusverlust auszulösen und den Vorgang abzubrechen, wenn Validierungsfehler gefunden werden. Sie benötigen diesen speziellen Code nicht per Knopfdruck.
MarkJ