Business Validation Logic-Code Geruch

StackOverflow https://stackoverflow.com/questions/635519

Frage

Betrachten Sie den folgenden Code ein: 

partial class OurBusinessObject {
    partial void OnOurPropertyChanged() {
        if(ValidateOurProperty(this.OurProperty) == false) {
            this.OurProperty = OurBusinessObject.Default.OurProperty;
        }
    }
}

Das heißt, wenn der Wert von OurProperty in OurBusinessObject geändert wird, wenn der Wert nicht gültig ist, stellen Sie den Standardwert zu sein. Dieses Muster scheint mir Code Geruch aber andere hier (bei meinem Arbeitgeber) nicht einverstanden. Was sind Ihre Gedanken?

Edited hinzufügen : Ich bin gebeten worden, für eine Erklärung hinzuzufügen, warum dieser Gedanke ist in Ordnung zu sein. Die Idee war, dass anstatt die Daten den Hersteller des Business-Objekts validieren, wobei das Business-Objekt seine eigenen Eigenschaften bestätigen konnte und sauber Standardwerte in Fällen festgelegt, wenn die Überprüfung fehlschlägt. Ferner wurde es gedacht, wenn die Validierungsregeln zu ändern, müssen die Business-Objekt-Produzenten nicht ihre Logik ändern, wie das Business-Objekt Pflege der Validierung und Reinigung der Daten stattfinden wird.

War es hilfreich?

Lösung

Es ist absolut schrecklich. Viel Glück versuchen, zu debuggen Probleme in der Produktion. Das einzige, was es führen kann, ist, Fehler zu decken, die gerade woanders auftauchen werden, wo es überhaupt nicht klar, wo sie herkommt.

Andere Tipps

Ich denke, dass ich mit Ihnen zustimmen. Dies könnte auf jeden Fall zu Problemen führen, wo die Logik unerwartet auf die Standardwerte zurückgibt, die zum Debuggen sehr schwierig sein könnte.

Am allerwenigsten, dieses Verhalten protokolliert werden soll, aber dies scheint eher wie ein Fall für eine Ausnahme zu werfen.

Für mich sieht aus wie das Symptom und nicht das eigentliche Problem. Was wirklich los ist, dass der Setter für OurProperty nicht den ursprünglichen Wert für die Verwendung in dem OnOurPropertyChanged Ereignisse zu bewahren. Wenn Sie das tun, plötzlich wird es leichter, bessere Entscheidungen darüber zu treffen, wie es weitergehen.

Für diese Angelegenheit, was Sie wirklich wollen, ist ein OnOurPropertyChanging Ereignis, das von dem Setter angehoben wird vor die Zuordnung tatsächlich erfolgt. So können Sie die Zuordnung in erster Linie erlauben oder verweigern können. Ansonsten gibt es eine kleine Menge an Zeit, in der das Objekt nicht gültig ist, und das bedeutet, dass der Typ nicht sicher ist, fädelt und Sie können nicht auf Konsistenz zählen, wenn Sie Sie betrachten Gleichzeitigkeit ein Anliegen ist.

Auf jeden Fall eine fragwürdige Praxis.

Wie würde ein ungültiger Wert überhaupt zu diesem Objekt zugewiesen werden? Wäre das nicht zeigen, es ist ein Fehler irgendwo im Telefonvorwahl, in dem Fall, dass Sie wahrscheinlich würde wollen sofort wissen? Oder dass eine Benutzereingabe etwas falsch in diesem Fall sie sollte sofort informiert werden?

In der Regel „failing schnell“ macht Aufspüren von Fehlern sind viel einfacher. Schweigend Zuordnung zu einem Standard hinter den Kulissen ist verwandt mit „Magie“ und wird nur Verwirrung stiften, wer die Code-Basis zu halten hat.

Distaste für den Begriff ‚Codegeruch‘ zur Seite, könnten Sie recht haben - je nachdem, wo es aus kam, leise den Wert zu ändern ist wahrscheinlich nicht eine gute Sache. Es wäre besser, Ihr Wert, um sicherzustellen, gültig ist, anstatt nur auf die Standardeinstellung zurückkehrt.

Ich würde es sehr empfehlen Refactoring, bevor Sie die Eigenschaft zu überprüfen.

könnten Sie haben immer eine Methode, die eher wie war: 

T GetValidValueForProperty<T>(T suggestedValue, T currentValue);

oder auch: 

T GetValidValueForProperty<T>(string propertyName, T suggestedValue, T currentValue);

Wenn Sie das tun, bevor Sie die Eigenschaft festlegen, können Sie es auf die Business-Logik passieren könnte zu validieren und die Business-Logik könnte die Standardeigenschaft Wert (Ihr aktuelles Verhalten) OR (vernünftigere in den meisten Fällen) zurück, zurück current~~POS=TRUNC, hatte so Einstellung keine Wirkung.

Dies würde verwendet werden, eher wie: 

T OurProperty
{
    get
    {
        return this.propertyBackingField;
    }
    set
    {
        this.propertyBackingField = this.GetValidValueForProperty(value, this.propertyBackingField);
    }
}

Es ist wirklich egal, was Sie tun, aber es ist wichtig zu überprüfen, bevor Sie Ihren aktuellen Wert ändern. Wenn Sie Ihren Wert ändern, bevor Sie bestimmen, ob der neue Wert ist gut, fordern Sie für Ärger auf lange Sicht.

Es kann oder auch nicht „riechen“, aber ich bin Neigung mehr in Richtung „Ja, es riecht“.

Hat Einstellung OurProperty auf den Standard haben einen logischen Grund für so tun oder ist es einfach bequemer so in Code zu tun? Es ist möglich, aber unwahrscheinlich, dass in der Praxis ein Szenario ausdenken, wo dies würde Verhalten erwarten, aber ich vermute, dass in den meisten Fällen sollen Sie es eine Ausnahme und Handhabung sauber irgendwo werfen.

Hat man den Wert auf Sie näher an die Standard oder Sie weg bewegen von der funktionelle Spezifikationen Beschreibung, wie die Anwendung funktionieren soll?

Sie sind die Validierung eine Änderung, nachdem es geschehen ist? Die Validierung sollte getan werden, bevor die Geschäftigkeit Eigenschaft verändert wird.

Ihre questing Answering: die Lösung in diesem Codeausschnitt dargestellt können große Probleme in der Produktion generieren, Sie wissen nicht, ob der Standardwert erschien aufgrund einer ungültigen Eingabe oder einfach nur, weil etwas anderes den Wert auf den Standardwert

Es ist schwer zu sagen, ohne den Kontext oder Geschäftsregeln zu kennen. Generell sind aber sprechen, sollten Sie nur zum Zeitpunkt der Eingabe validieren und vielleicht einmal vor Ausdauer, aber die Art und Weise Sie es tun wird nicht wirklich erlauben Sie zu überprüfen, da Sie nicht eine Eigenschaft enthält einen ungültigen Wert ermöglicht.

ich glaube, Ihre Validierungslogik sollte eine Ausnahme auslösen, wenn gefragt einen ungültigen Wert zu verwenden. Wenn Ihr Verbraucher einen Standardwert verwenden möchte, sollte es darum bitten ausdrücklich, entweder durch einen speziellen, dokumentiert Wert oder durch eine andere Methode.

Die einzige Art von Ausnahmen Ich denke, kann verzeihlich wäre, wäre, wie, Fall Normalisierung, wie in E-Mail-Felder Duplikate zu erkennen.

Außerdem, warum in der Welt ist dies teilweise? Mit partiellen Klassen für alles andere als ein Code Framework erzeugt selbst ist ein codesmell da Sie mit ihnen wahrscheinlich Komplexität zu verstecken, die sowieso aufgespalten werden sollte!

ich mit Grzenio einverstanden und möchte hinzufügen, dass der beste Weg, um einen Fehler bei der Überprüfung nach unten in der Domänenschicht (auch bekannt als Business-Objekte) zu handhaben ist, eine Ausnahme zu erzeugen. Diese Ausnahme könnte den ganzen Weg bis in die UI-Ebene ausbreiten, wo sie behandelt werden könnten und interaktiv mit dem Benutzer behoben. Allerdings, je nach den Fähigkeiten und Technologien beteiligt, kann dies nicht immer möglich sein, wobei in diesem Fall, werden Sie wahrscheinlich sein sollten, in der UI-Ebene (möglicherweise zusätzlich zu der Domänenschicht) Validierung auf. Es ist weniger als ideal, aber vielleicht die einzige sinnvolle Option sein. In jedem Fall ist es auf einen Standardwert Einstellung ist eine schreckliche Sache zu tun und wird auf subtile Bugs führen, die in der Nähe unmöglich sein werden, zu diagnostizieren. Wenn auf breiter Ebene durchgeführt, erhalten Sie eine wartbaren System in kürzester Zeit (vor allem, wenn Sie keine Unit-Tests haben Sichern Sie oben).

Ein Argument, das ich gegen diese haben, ist die folgende. Angenommen, der Benutzer / Hersteller des Business-Objekts versehentlich einen ungültigen Wert eingibt. Dann wird dieses Muster beschönigt diese Tatsache und Standarddaten zu reinigen. Aber der richtige Weg, dies zu handhaben ist, einen Fehler zu werfen und haben die Anwender / Hersteller überprüfen / reinigen ihre Eingangsdaten.

Ich würde sagen, Property implementieren und die Geschäftslogik ermöglichen, einen Wert zu genehmigen / ablehnen und dann danach, eine Ausnahme für ungültige Werte werfen.

Auf diese Weise haben Sie nicht immer einen ungültigen Wert. Das, und Sie sollten nie ein Benutzerinformationen ändern. Was passiert, wenn ein Benutzer fügt die Datenbank einen Eintrag, und hält für seine eigenen Aufzeichnungen davon Spur? Ihr Code würde den Wert auf den Standard neu zuweisen, und jetzt ist er die falschen Informationen zu verfolgen. Es ist besser so schnell wie möglich die Benutzer zu informieren.

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top