Refactoring einer Schaltfläche Aktivieren/Deaktivieren Umschaltfunktion

Question

Ich habe die folgende einfache Funktion:

private void EnableDisable941ScheduleBButton() 
    { 
     if (this._uosDepositorFrequency.Value != null) 
      this._btnScheduleB.Enabled = ((int)this._uosDepositorFrequency.Value == 0); 
    } 

Es ist ein Mitglied einer Winform Klasse, die ich versuche, in eine passive Ansicht und Moderator zu spalten. Es ist offensichtlich, dass Business-Logik mit UI-Verkabelung verwickelt ist. Ich bin mir einfach nicht sicher, wie ich sie am besten trennen könnte.

Um einen kleinen Kontext zu geben, wird die Funktion von drei Stellen im Formular aufgerufen. _uosDepositorFrequency ist eine Radiobutton-Gruppe mit nur zwei Tasten.

Irgendwelche Ideen?

Update:

Ok, vielleicht ist es nicht so offensichtlich, wie ich dachte. Die Geschäftsregel besagt, dass, wenn ein Arbeitgeber halbwöchentliche Einzahlungen tätigt (_uosDepositorFrequency.Value = 0), sie ein Schedule B Formular ausfüllen müssen.

Answer 1

Zuerst möchte ich allen danken, die auf meine Frage geantwortet haben.

Ich habe einige Zeit damit verbracht und ich glaube, ich habe eine Lösung gefunden.

Zuerst habe ich _uosDepositorFrequency.Value und _btnScheduleB.Enabled als öffentliche Eigenschaften verfügbar gemacht und die Ansichtsschnittstelle aktualisiert. Ich habe auch ein paar Minuten gebraucht, um eine Enum für die Einzahlungsfrequenzen zu definieren.

public bool EnableScheduleB 
    { 
     get 
     { 
      return _btnScheduleB.Enabled; 
     } 
     set 
     { 
      _btnScheduleB.Enabled = value; 
     } 
    } 

    public DepositFrequency DepositorFrequency 
    { 
     get 
     { 
      return (DepositFrequency)_uosDepositorFrequency.Value; 
     } 
     set 
     { 
      _uosDepositorFrequency.Value = (int)value; 
     } 
    } 

Dann kopierte ich den Körper der ursprünglichen Funktion zu meinem Moderator und modifiziert es um die Eigenschaften zu verwenden Ich habe gerade. Die Nullprüfung in der ursprünglichen Funktion erwies sich als unnötig, da das Steuerelement _uosDepositorFrequency an anderer Stelle initialisiert wird.

public void UpdateScheduleBStatus() 
    { 
     ReturnView.EnableScheduleB = ReturnView.DepositorFrequency == DepositFrequency.Semiweekly; 
    } 

Schließlich wird der Event-Handler _uosDepositorFrequency_ValueChanged wurde aktualisiert UpdateScheduleBStatus zu nennen.

private void _uosDepositorFrequency_ValueChanged(object sender, System.EventArgs e) 
    { 
     Presenter.UpdateScheduleBStatus(); 
    } 

Kommentare willkommen.

Answer 2

Ich glaube nicht, dass das Geschäftslogik ist. Es sieht für mich wie UI-Logik aus. Ich würde es nicht ändern.

Obwohl, wenn ich wpf verwenden würde, würde ich den aktivierten Zustand an die Daten binden.

Answer 3

Vielleicht vermisse ich etwas, aber ich bin mir nicht sicher, ob ich die Geschäftslogik dort sehe. Sie aktivieren/deaktivieren nur eine Schaltfläche basierend darauf, ob ein anderes Steuerelement im Formular einen Wert hat oder nicht. Ich sehe nicht die Notwendigkeit, diese Methode umzuformen, es ist alles Sichtlogik für mich.

Answer 4

Es klingt, als ob Sie die UI-Logik und die Geschäftsregel als eine Sache verwechselt haben. Der Code, den Sie haben, ist UI-Logik und sollte nicht refaktoriert werden. Die Geschäftsregel, die Sie sagen, ist, dass, wenn _btnScheduleB einen Wert von 0 hat, dann müssen Sie den Benutzer das Formular ausfüllen. Ihre Geschäftslogik sollte sicherstellen, dass Sie ein ausgefülltes Formular haben, wenn _btnScheduleB aktiviert ist, bevor der Benutzer fortfahren kann.

Answer 5

Moderator:

if(this._uosDepositorFrequency.Value > 0) //int objects cannot be null 
     ViewData["ScheduleBRequired"] = true; 

Ausblick:

private  void Draw() 
    { 
      if ((bool)ViewData["ScheduleBRequired"]){ 
        this._btnScheduleB.Enabled = true; 
        this._validatorScheduleB.Active = true; //required data should be checked clientside with js 
      } 
    } 

Wenn es ein businessrequirement ist, dass diese gefüllt werden soll, sollte es durch die Moderatorin ausgelöst werden. Das UI ist verantwortlich für die Befolgung der Entscheidungen des Präsentators. z.B. um den ScheduleB zu verlangen oder nicht ....

Answer 6

Eine Alternative (und ich sage nicht, dass es eine bessere ist) ist die Auswahl geändert Ereignis für die Funksteuerung das Modell aktualisieren (eine Arbeitgeber-Entität?), die Daraufhin wird eine Art von DepositorChanged-Ereignis ausgelöst, das von der Benutzeroberfläche abonniert wird, und die Zeitplanschaltfläche wird entsprechend aktiviert/deaktiviert. Das Beobachtermuster, mehr oder weniger.

Die obige Alternative erzeugt viel mehr Komplexität und ich würde vorschlagen, dies dem UI-Code zu überlassen (fügen Sie einen Kommentar dazu hinzu oder verwenden Sie eine zwischengeschaltete boolesche Variable, um dem Leser die Absicht zu erklären). Vor allem, wenn dies der einzige Ort ist, an dem die Regel angewendet wird.

Eine Randnotiz in Bezug auf den Code: Es ist eine religiöse Debatte zwischen den Klassenfeldern Präfix mit Unterstrichen oder this. statt. Ich denke, jeder stimmt zu, dass beides überflüssig ist ...

Answer 7

Wie die anderen darauf hingewiesen haben, hält dies Ihre Benutzeroberfläche konsistent, so dass es wirklich keine Geschäftslogik ist. Sie können den Null-Check jedoch so entfernen.

this._btnScheduleB.Enabled = (int)(this._uosDepositorFrequency.Value ?? 0) == 0; 

Answer 8

würde ich nicht zu viele Sorgen über 2 Zeilen Code Refactoring es sei denn, es wirklich keinen Sinn im aktuellen Format gemacht hat.