Konstruktor oder Setter-Methode verwenden?

16

Ich arbeite an einem UI-Code, in dem ich eine ActionKlasse habe.

public class MyAction extends Action {
    public MyAction() {
        setText("My Action Text");
        setToolTip("My Action Tool tip");
        setImage("Some Image");
    }
}

Bei der Erstellung dieser Action-Klasse wurde davon ausgegangen, dass die ActionKlasse nicht anpassbar ist (in gewissem Sinne - der Text, der Tooltip oder das Bild werden an keiner Stelle im Code geändert). Jetzt müssen wir den Aktionstext an einer bestimmten Stelle im Code ändern. Daher schlug ich meinem Kollegen vor, den fest codierten Aktionstext aus dem Konstruktor zu entfernen und als Argument zu akzeptieren, damit jeder gezwungen ist, den Aktionstext zu übergeben. So etwas wie dieser Code unten -

public class MyAction extends Action {
    public MyAction(String actionText) {
        setText(actionText);
        setTooltip("My Action tool tip"); 
        setImage("My Image"); 
    }
}

Er ist jedoch der Ansicht, dass die setText()Methode , da sie zur Basisklasse gehört, flexibel verwendet werden kann, um den Aktionstext überall dort zu übergeben, wo eine Aktionsinstanz erstellt wird. Auf diese Weise muss die vorhandene MyActionKlasse nicht geändert werden . Also würde sein Code ungefähr so ​​aussehen.

MyAction action = new MyAction(); //this creates action instance with the hardcoded text
action.setText("User required new action text"); //overwrite the existing text.

Ich bin mir nicht sicher, ob dies der richtige Weg ist, um mit dem Problem umzugehen. Ich denke, in dem oben genannten Fall wird der Benutzer den Text trotzdem ändern. Warum sollte er ihn nicht zwingen, während er die Aktion erstellt? Der einzige Vorteil, den ich mit dem Originalcode sehe, ist, dass der Benutzer eine Action-Klasse erstellen kann, ohne viel über das Festlegen von Text nachdenken zu müssen.

zswap
quelle
1
Die Sprache, die Sie verwenden, lässt das Überladen von Konstruktoren nicht zu?
Mat
1
Ich benutze Java.So Ja, es erlaubt und ich denke, das könnte ein Weg sein, um damit umzugehen
zswap
2
Ich würde bemerken, dass Ihre Klasse effektiv unveränderlich ist, wenn Sie keine öffentliche Möglichkeit haben, Klassenmitglieder nachträglich festzulegen . Indem Sie einen öffentlichen Setter zulassen , wird Ihre Klasse jetzt veränderlich , und Sie müssen dies möglicherweise berücksichtigen, wenn Sie von der Unveränderlichkeit abhängen.
cbojar
Ich würde sagen, wenn es gesetzt werden muss, damit Ihr Objekt gültig ist, setzen Sie es in jeden Konstruktor ... wenn es optional ist (hat einen vernünftigen Standard) und Sie sich nicht um Unveränderlichkeit kümmern, setzen Sie es in einen Setter. Es sollte unmöglich sein, ein Objekt in einen ungültigen Zustand zu versetzen oder es nach der Versetzung in einen ungültigen Zustand zu versetzen, wo immer dies möglich ist.
Bill K

Antworten:

15

Der einzige Vorteil, den ich mit dem Originalcode sehe, ist, dass der Benutzer eine Action-Klasse erstellen kann, ohne viel über das Festlegen von Text nachdenken zu müssen.

Das ist eigentlich kein Vorteil, für die meisten Zwecke ist es ein Nachteil und in den übrigen Fällen würde ich es als Gleichstand bezeichnen. Was passiert, wenn jemand vergisst, setText () nach der Erstellung aufzurufen? Was ist, wenn dies in einem ungewöhnlichen Fall der Fall ist, vielleicht ein Fehlerbehandler? Wenn Sie das Festlegen des Texts wirklich erzwingen möchten, müssen Sie es beim Kompilieren erzwingen, da nur Fehler beim Kompilieren tatsächlich schwerwiegend sind . Alles, was zur Laufzeit passiert, hängt von dem jeweiligen Codepfad ab, der ausgeführt wird.

Ich sehe zwei klare Wege vorwärts:

  1. Verwenden Sie einen Konstruktorparameter, wie Sie vorschlagen. Wenn Sie wirklich wollen, können Sie nulleine leere Zeichenfolge übergeben, aber die Tatsache, dass Sie keinen Text zuweisen, ist eher explizit als implizit. Es ist einfach, die Existenz eines nullParameters zu erkennen und zu sehen, dass möglicherweise einige Überlegungen angestellt wurden, aber nicht so einfach, das Fehlen eines Methodenaufrufs zu erkennen und festzustellen, ob das Fehlen eines solchen beabsichtigt war oder nicht. Für einen einfachen Fall wie diesen ist dies wahrscheinlich der Ansatz, den ich wählen würde.
  2. Verwenden Sie ein Factory / Builder-Muster. Dies mag für ein so einfaches Szenario zu viel sein, ist jedoch im Allgemeinen sehr flexibel, da Sie eine beliebige Anzahl von Parametern festlegen und die Voraussetzungen vor oder während der Objektinstanziierung überprüfen können (wenn das Erstellen des Objekts eine große Operation ist und / oder oder die Klasse kann auf mehrere Arten verwendet werden, dies kann ein großer Vorteil sein). Insbesondere in Java ist dies auch eine verbreitete Redewendung, und es ist sehr selten eine schlechte Sache, in der von Ihnen verwendeten Sprache und im verwendeten Framework festgelegten Mustern zu folgen.
ein CVn
quelle
10

Konstruktorüberladung wäre hier eine einfache und unkomplizierte Lösung:

public class MyAction extends Action {
    public MyAction(String actionText) {
        setText(actionText);
        setTooltip("My Action tool tip"); 
        setImage("My Image"); 
    }
    public MyAction() {
        this("My Action Text");
    }
}

Es ist besser als .setTextspäter anzurufen , da auf diese Weise nichts überschrieben werden muss, actionTextwas von Anfang an beabsichtigt sein kann.

Wenn sich Ihr Code weiterentwickelt und Sie noch mehr Flexibilität benötigen (was sicherlich passieren wird), profitieren Sie von dem Factory / Builder-Muster, das in einer anderen Antwort vorgeschlagen wird.

janos
quelle
Was passiert, wenn eine zweite Eigenschaft angepasst werden soll?
Kevin Cline
3
Für eine 2nd, 3rd, .. -Eigenschaft können Sie dieselbe Technik anwenden. Je mehr Eigenschaften Sie jedoch anpassen möchten, desto unhandlicher wird dies. Irgendwann wird es sinnvoller sein, ein Factory / Builder-Muster zu implementieren, wie auch @ michael-kjorling in seiner Antwort sagte.
Januar,
6

Fügen Sie eine fließende 'setText'-Methode hinzu:

public class MyAction ... {
  ...
  public MyAction setText(String text) { ... ; return this; }
}

MyAction a = new MyAction().setText("xxx");

Was könnte klarer sein als das? Wenn Sie eine weitere anpassbare Eigenschaft hinzufügen möchten, ist dies kein Problem.

Kevin Cline
quelle
+1, stimme ich zu und fügte eine weitere Antwort hinzu, die mit weiteren Eigenschaften ergänzt wird. Ich denke, die flüssige API ist klarer zu verstehen, wenn Sie mehr als eine einzelne Eigenschaft als Beispiel haben.
Machado
Ich mag fließende Schnittstellen, besonders für Bauherren, die unveränderliche Objekte herstellen! Je mehr Parameter, desto besser funktioniert das. setText()Wenn Sie sich jedoch das spezifische Beispiel in dieser Frage ansehen, vermute ich, dass es für die Action-Klasse definiert ist, von der MyAction erbt. Es hat wahrscheinlich bereits einen ungültigen Rückgabetyp.
GlenPeterson
1

Genau wie Kevin Cline in seiner Antwort sagte, denke ich, dass der Weg dahin darin besteht, eine flüssige API zu erstellen . Ich möchte nur hinzufügen, dass die flüssige API besser funktioniert, wenn Sie mehr als eine Eigenschaft haben, die Sie möglicherweise verwenden.

Es wird Ihren Code besser lesbar, und aus meiner Sicht einfacher und macht aham , „sexy“ zu schreiben.

In Ihrem Fall würde es so verlaufen (Entschuldigung für einen Tippfehler, es ist ein Jahr her, seit ich mein letztes Java-Programm geschrieben habe):

 public class MyAction extends Action {
    private String _text     = "";
    private String _tooltip  = "";
    private String _imageUrl = "";

    public MyAction()
    {
       // nothing to do here.
    }

    public MyAction text(string value)
    {
       this._text = value;
       return this;
    }

    public MyAction tooltip(string value)
    {
       this._tooltip = value;
       return this;
    }

    public MyAction image(string value)
    {
       this._imageUrl = value;
       return this;
    }
}

Und die Verwendung wäre wie folgt:

MyAction action = new MyAction()
    .text("My Action Text")
    .tooltip("My Action Tool tip")
    .image("Some Image");
Machado
quelle
Schlechte Idee, was ist, wenn sie vergessen, Text oder etwas Wichtiges zu setzen.
Prakhar
1

Der Rat, Konstrukteure oder Bauherren zu verwenden, ist im Allgemeinen in Ordnung, aber meiner Erfahrung nach fehlen einige wichtige Punkte für Aktionen, die

  1. Müssen möglicherweise internationalisiert werden
  2. Wahrscheinlich wird sich das Marketing in letzter Minute ändern.

Es wird dringend empfohlen, den Namen, den Tooltip, das Symbol usw. aus einer Eigenschaftendatei, XML usw. zu lesen. Für die Aktion "Datei öffnen" könnten Sie beispielsweise Eigenschaften übergeben, nach denen gesucht wird

File.open.name=Open
File.open.tooltip=Open a file
File.open.icon=somedir/open.jpg

Dies ist ein Format, das ziemlich einfach ins Französische zu übersetzen ist, um ein neues besseres Symbol auszuprobieren usw. Ohne Programmiererzeit oder Neukompilierung.

Dies ist nur ein grober Abriss, dem Leser bleibt viel übrig ... Suchen Sie nach anderen Beispielen für Internationalisierung.

user949300
quelle
0

Es ist sinnlos, setText (actionText) oder setTooltip ("My action tool tip") im Konstruktor aufzurufen. Es ist einfacher (und Sie erzielen mehr Leistung), wenn Sie das entsprechende Feld direkt initialisieren:

    public MyAction(String actionText) {
        this.actionText = actionText;
    }

Wenn Sie actionText während der Lebensdauer des MyAction-entsprechenden Objekts ändern, sollten Sie eine Setter-Methode platzieren. Wenn nicht, initialisieren Sie das Feld nur im Konstruktor, ohne eine Setter-Methode anzugeben.

Da QuickInfo und Bild Konstanten sind, behandeln Sie sie als Konstanten. Felder haben:

private (or even public) final static String TOOLTIP = "My Action Tooltip";

Tatsächlich ist es beim Entwerfen allgemeiner Objekte (keine Beans oder Objekte, die ausschließlich Datenstrukturen darstellen) eine schlechte Idee, Setter und Getter bereitzustellen, da diese die Kapselung aufheben.

m3th0dman
quelle
4
Jeder halbwegs kompetente Compiler und JITter sollte die setText () etc-Aufrufe inline setzen, daher sollte der Leistungsunterschied zwischen einem Funktionsaufruf zum Ausführen einer Zuweisung und der Zuweisung anstelle des Funktionsaufrufs höchstens vernachlässigbar und wahrscheinlicher sein als nicht Null.
CVn
0

Ich denke, dass dies zutrifft, wenn wir eine allgemeine Aktionsklasse erstellen (wie update, die zum Aktualisieren von Employee, Department ... verwendet wird). Es hängt alles vom Szenario ab. Wenn eine bestimmte Aktionsklasse (z. B. "Mitarbeiter aktualisieren") (die an vielen Stellen in der Anwendung verwendet wird - Mitarbeiter aktualisieren) mit der Absicht erstellt wird, an jeder Stelle in der Anwendung denselben Text, dieselbe QuickInfo und dasselbe Bild beizubehalten (aus Gründen der Konsistenz). So können Texte, QuickInfos und Bilder hartcodiert werden, um die Standardtexte, QuickInfos und Bilder bereitzustellen. Um noch mehr Flexibilität zu bieten und diese anzupassen, sollte es entsprechende Setter-Methoden geben. Wenn wir nur 10% der Stellen im Kopf behalten, müssen wir sie ändern. Wenn der Benutzer jedes Mal einen Aktionstext entgegennimmt, kann dies jedes Mal zu einem anderen Text für dieselbe Aktion führen. Wie 'Update Emp', 'Update Employee', 'Change Employee' oder 'Edit Employee'.

Sandig
quelle
Ich denke, überladener Konstruktor sollte das Problem noch lösen. Weil Sie in allen "10%" -Fällen zuerst eine Aktion mit Standardtext erstellen und dann den Aktionstext mit der Methode "setText ()" ändern. Warum setzen Sie nicht den entsprechenden Text während der Erstellung der Aktion?
zswap
0

Überlegen Sie, wie Instanzen verwendet werden, und verwenden Sie eine Lösung, die die Benutzer anleitet oder sogar zwingt, diese Instanzen auf die richtige oder zumindest beste Weise zu verwenden. Ein Programmierer, der diese Klasse verwendet, muss sich über viele andere Dinge Gedanken machen. Diese Klasse sollte nicht zur Liste hinzugefügt werden.

Wenn die MyAction-Klasse beispielsweise nach der Erstellung (und möglicherweise nach einer anderen Initialisierung) unveränderlich sein soll, sollte sie keine Setter-Methode haben. Wenn die meiste Zeit der Standard "Mein Aktionstext" verwendet wird, sollte es einen parameterlosen Konstruktor sowie einen Konstruktor geben, der optionalen Text zulässt. Jetzt muss der Benutzer nicht mehr darüber nachdenken, die Klasse in 90% der Fälle korrekt zu verwenden. Wenn der Benutzer in der Regel sollte sich Gedanken über den Text geben, lassen Sie die parameterlosen Konstruktor. Jetzt muss der Benutzer nachdenken, wenn es nötig ist, und kann einen notwendigen Schritt nicht übersehen.

Wenn eine MyActionInstanz nach der vollständigen Erstellung veränderbar sein soll, benötigen Sie einen Setter für den Text. Es ist verlockend, das Setzen des Wertes im Konstruktor zu überspringen (DRY-Prinzip - "Wiederholen Sie sich nicht"), und wenn der Standardwert normalerweise gut genug ist, würde ich das tun. Wenn dies nicht der Fall ist, wird der Benutzer durch die Anforderung des Textes im Konstruktor gezwungen, zu überlegen, wann er sollte.

Beachten Sie, dass diese Benutzer nicht dumm sind . Sie haben einfach zu viele echte Probleme, um die sie sich Sorgen machen müssen. Indem Sie über die "Schnittstelle" Ihrer Klasse nachdenken, können Sie verhindern, dass diese auch zu einem echten Problem wird - und zu einem unnötigen.

RalphChapin
quelle
0

In der folgenden vorgeschlagenen Lösung ist die Superklasse abstrakt und für alle drei Elemente ist ein Standardwert festgelegt.

Die Unterklasse hat verschiedene Konstruktoren, so dass der Programmierer sie instanziieren kann.

Wenn der erste Konstruktor verwendet wird, haben alle Member die Standardwerte.

Wenn der zweite Konstruktor verwendet wird, geben Sie dem actionText-Member einen Anfangswert und lassen die anderen beiden Member mit dem Standardwert ...

Wenn der dritte Konstruktor verwendet wird, instanziieren Sie ihn mit einem neuen Wert für actionText und toolTip, wobei imageURl mit dem Standardwert belassen wird ...

Und so weiter.

public abstract class Action {
    protected String text = "Default action text";
    protected String toolTip = "Default action tool tip";
    protected String imageURl = "http://myserver.com/images/default.png";

    .... rest of code, I guess setters and getters
}

public class MyAction extends Action {


    public MyAction() {

    }

    public MyAction(String actionText) {
        setText(actionText);
    }

    public MyAction(String actionText, String toolTip_) {
        setText(actionText);
        setToolTip(toolTip_);   
    }

    public MyAction(String actionText, String toolTip_; String imageURL_) {
        setText(actionText);
        setToolTip(toolTip_);
        setImageURL(imageURL_);
    }


}
Tulains Córdova
quelle