Gibt es jemals einen Grund, die gesamte Arbeit eines Objekts in einem Konstruktor auszuführen?

Lassen Sie mich dies vorwegnehmen, indem Sie sagen, dass dies weder mein Code noch der Code meiner Mitarbeiter ist. Vor Jahren, als unser Unternehmen noch kleiner war, mussten wir einige Projekte durchführen, für die wir nicht die Kapazität hatten, sodass sie ausgelagert wurden. Jetzt habe ich nichts gegen Outsourcing oder Auftragnehmer im Allgemeinen, aber die Codebasis, die sie produzierten, ist eine Masse von WTFs. Davon abgesehen funktioniert es (meistens), also schätze ich, dass es in den Top 10% der ausgelagerten Projekte liegt, die ich gesehen habe.

Da unser Unternehmen gewachsen ist, haben wir versucht, mehr von unserer Entwicklung in Eigenregie zu übernehmen. Dieses spezielle Projekt ist in meinem Schoß gelandet, also habe ich es durchgesehen, aufgeräumt, Tests hinzugefügt usw. usw.

Es gibt ein Muster, das sich oft wiederholt, und es scheint so überwältigend schrecklich, dass ich mich gefragt habe, ob es vielleicht einen Grund gibt und ich es einfach nicht sehe. Das Muster ist ein Objekt ohne öffentliche Methoden oder Member, nur ein öffentlicher Konstruktor, der die gesamte Arbeit des Objekts erledigt.

Zum Beispiel (der Code ist in Java, wenn das wichtig ist, aber ich hoffe, dass dies eine allgemeinere Frage ist):

public class Foo {
  private int bar;
  private String baz;

  public Foo(File f) {
    execute(f);
  }

  private void execute(File f) {
     // FTP the file to some hardcoded location, 
     // or parse the file and commit to the database, or whatever
  }
}

Wenn Sie sich fragen, wird diese Art von Code häufig folgendermaßen aufgerufen:

for(File f : someListOfFiles) {
   new Foo(f);
}

Nun wurde mir vor langer Zeit beigebracht, dass instanziierte Objekte in einer Schleife im Allgemeinen eine schlechte Idee sind und dass Konstruktoren ein Minimum an Arbeit leisten sollten. Wenn Sie sich diesen Code ansehen, ist es besser, den Konstruktor zu löschen und executeeine öffentliche statische Methode zu erstellen .

Ich habe den Auftragnehmer gefragt, warum es so gemacht wurde, und die Antwort lautete: "Wir können es ändern, wenn Sie wollen." Welches war nicht wirklich hilfreich.

Wie auch immer, gibt es jemals einen Grund, so etwas in irgendeiner Programmiersprache zu tun, oder ist dies nur eine weitere Einreichung beim Daily WTF?

Ok, gehe die Liste durch:

Mir wurde vor langer Zeit beigebracht, dass instanziierte Objekte in einer Schleife im Allgemeinen eine schlechte Idee sind

Nicht in irgendwelchen Sprachen, die ich benutzt habe.

In C ist es eine gute Idee, Ihre Variablen im Voraus zu deklarieren, aber das unterscheidet sich von dem, was Sie gesagt haben. Es kann etwas schneller sein, wenn Sie Objekte über der Schleife deklarieren und wiederverwenden, aber es gibt viele Sprachen, in denen diese Geschwindigkeitssteigerung bedeutungslos ist (und wahrscheinlich einige Compiler, die die Optimierung für Sie durchführen :)).

Wenn Sie ein Objekt in einer Schleife benötigen, erstellen Sie im Allgemeinen ein Objekt.

Konstrukteure sollten ein Minimum an Arbeit leisten

Konstruktoren sollten die Felder eines Objekts instanziieren und alle anderen Initialisierungen vornehmen, die erforderlich sind, um das Objekt einsatzbereit zu machen. Dies bedeutet im Allgemeinen, dass Konstruktoren klein sind, aber es gibt Szenarien, in denen dies einen erheblichen Arbeitsaufwand bedeuten würde.

Gibt es jemals einen Grund, so etwas in irgendeiner Programmiersprache zu tun, oder ist dies nur eine weitere Einreichung beim Daily WTF?

Es ist eine Einreichung bei der Daily WTF. Zugegeben, es gibt schlimmere Dinge, die Sie mit Code tun können. Das Problem ist, dass der Autor ein großes Missverständnis darüber hat, was Klassen sind und wie man sie verwendet. Im Einzelnen sehe ich Folgendes als falsch an diesem Code:

• Klassenmissbrauch: Die Klasse verhält sich grundsätzlich wie eine Funktion. Wie Sie bereits erwähnt haben, sollte es entweder durch eine statische Funktion ersetzt werden, oder die Funktion sollte nur in der Klasse implementiert werden, die sie aufruft. Kommt darauf an, was es macht und wo es eingesetzt wird.
• Mehraufwand für die Leistung: Je nach Sprache kann das Erstellen eines Objekts langsamer sein als das Aufrufen einer Funktion.
• Allgemeine Verwirrung: Für den Programmierer ist die Verwendung dieses Codes im Allgemeinen verwirrend. Ohne die Verwendung zu sehen, würde niemand wissen, wie der Autor den fraglichen Code verwenden wollte.

Für mich ist es eine kleine Überraschung, wenn ein Objekt im Konstruktor viel Arbeit leistet. Deshalb würde ich nur empfehlen, es abzulehnen (Prinzip der geringsten Überraschung).

Ein Versuch, ein gutes Beispiel zu geben :

Ich bin mir nicht sicher, ob es sich bei dieser Verwendung um ein Anti-Pattern handelt, aber in C # habe ich Code gesehen, der in etwa so lautete (etwas erfundenes Beispiel):

using (ImpersonationContext administrativeContext = new ImpersonationContext(ADMIN_USER))
{
    // Perform actions here under the administrator's security context
}
// We're back to "normal" privileges here

In diesem Fall ImpersonationContexterledigt die Klasse ihre gesamte Arbeit im Konstruktor und in der Dispose-Methode. Es nutzt die C # using-Anweisung, die von C # -Entwicklern recht gut verstanden wird, und stellt somit sicher, dass das Setup, das im Konstruktor ausgeführt wird, zurückgesetzt wird, sobald wir uns außerhalb der usingAnweisung befinden, selbst wenn eine Ausnahme auftritt. Dies ist wahrscheinlich die beste Verwendung, die ich dafür gesehen habe (dh "Arbeit" im Konstruktor), obwohl ich immer noch nicht sicher bin, ob ich sie für "gut" halte. In diesem Fall ist es ziemlich selbsterklärend, funktional und prägnant.

Ein Beispiel für ein gutes und ähnliches Muster wäre das Befehlsmuster . Sie führen die Logik dort definitiv nicht im Konstruktor aus, aber es ist in dem Sinne ähnlich, dass die gesamte Klasse einem Methodenaufruf entspricht (einschließlich der zum Aufrufen der Methode erforderlichen Parameter). Auf diese Weise können Informationen zum Methodenaufruf serialisiert oder für eine spätere Verwendung übergeben werden, was immens hilfreich ist. Vielleicht haben Ihre Auftragnehmer etwas Ähnliches versucht, obwohl ich das sehr bezweifle.

Kommentare zu Ihrem Beispiel:

Ich würde sagen, es ist ein sehr eindeutiges Anti-Pattern. Es fügt nichts hinzu, widerspricht den Erwartungen und führt eine übermäßig lange Verarbeitung im Konstruktor durch (FTP im Konstruktor? WTF in der Tat, obwohl man bekanntermaßen Webdienste auf diese Weise aufruft).

Ich habe Mühe, das Zitat zu finden, aber Grady Boochs Buch "Object Solutions" enthält eine Anekdote über ein Team von C-Entwicklern, die auf C ++ umsteigen. Anscheinend hatten sie eine Schulung absolviert, und das Management sagte ihnen, dass sie unbedingt "objektorientiert" vorgehen müssten. Um herauszufinden, was falsch ist, verwendete der Autor ein Code-Metrik-Tool und stellte fest, dass die durchschnittliche Anzahl der Methoden pro Klasse genau 1 betrug und Variationen des Ausdrucks "Do It" waren. Ich muss sagen, ich bin noch nie auf dieses spezielle Problem gestoßen, aber es kann anscheinend ein Symptom dafür sein, dass Menschen gezwungen sind, objektorientierten Code zu erstellen, ohne wirklich zu verstehen, wie und warum.

Nein.

Wenn die gesamte Arbeit im Konstruktor erledigt ist, bedeutet dies, dass das Objekt überhaupt nicht benötigt wurde. Höchstwahrscheinlich hat der Auftragnehmer das Objekt lediglich für die Modularität verwendet. In diesem Fall hätte eine nicht instanziierte Klasse mit einer statischen Methode den gleichen Vorteil erzielt, ohne überflüssige Objektinstanzen zu erstellen.

Es gibt nur einen Fall, in dem die gesamte Arbeit in einem Objektkonstruktor akzeptabel ist. Das ist, wenn der Zweck des Gegenstandes spezifisch ist, eine langfristige eindeutige Identität darzustellen, anstatt Arbeit durchzuführen. In einem solchen Fall würde das Objekt aus anderen Gründen als der Ausführung einer sinnvollen Arbeit in der Nähe gehalten. Zum Beispiel eine Singleton-Instanz.

Ich habe sicherlich viele Klassen erstellt, die viel Arbeit leisten, um etwas im Konstruktor zu berechnen, aber das Objekt ist unveränderlich, sodass die Ergebnisse dieser Berechnung über Eigenschaften verfügbar sind und dann nie etwas anderes ausgeführt werden. Das ist normale Unveränderlichkeit in Aktion.

Ich denke, die seltsame Sache hier ist, Code mit Nebenwirkungen in einen Konstruktor zu setzen. Ein Objekt zu erstellen und wegzuwerfen, ohne auf die Eigenschaften oder Methoden zuzugreifen, sieht seltsam aus (aber zumindest in diesem Fall ist es ziemlich offensichtlich, dass etwas anderes vor sich geht).

Dies wäre besser, wenn die Funktion in eine öffentliche Methode verschoben und dann eine Instanz der Klasse injiziert und die Methode von der for-Schleife wiederholt aufgerufen würde.

Gibt es jemals einen Grund, die gesamte Arbeit eines Objekts in einem Konstruktor auszuführen?

Nein.

• Ein Konstruktor sollte keine Nebenwirkungen haben.
  • Alles andere als die Initialisierung privater Felder sollte als Nebeneffekt angesehen werden.
  • Ein Konstruktor mit Nebenwirkungen bricht das Single-Responsibilty-Prinzip (SRP) und widerspricht dem Geist der objektorientierten Programmierung (OOP).
• Ein Konstruktor sollte leicht sein und niemals versagen.
  • Zum Beispiel schaudere ich immer, wenn ich einen Try-Catch-Block in einem Konstruktor sehe. Ein Konstruktor sollte keine Ausnahmen auslösen oder Fehler protokollieren.

Man könnte diese Richtlinien vernünftigerweise in Frage stellen und sagen: "Aber ich halte mich nicht an diese Regeln und mein Code funktioniert einwandfrei!" Darauf würde ich antworten: "Nun, das mag wahr sein, bis es nicht mehr so ​​ist."

• Ausnahmen und Fehler innerhalb eines Konstruktors sind sehr unerwartet. Wenn sie nicht dazu aufgefordert werden, werden zukünftige Programmierer nicht geneigt sein, diese Konstruktoraufrufe mit defensivem Code zu umgeben.
• Wenn in der Produktion ein Fehler auftritt, kann es schwierig sein, die generierte Stapelverfolgung zu analysieren. Der obere Rand der Stapelablaufverfolgung zeigt möglicherweise auf den Konstruktoraufruf, aber im Konstruktor passieren viele Dinge, und er zeigt möglicherweise nicht auf das tatsächliche LOC, bei dem ein Fehler aufgetreten ist.
  • Ich habe viele .NET-Stack-Traces analysiert, bei denen dies der Fall war.

Es scheint mir, dass die Person, die dies getan hat, versucht hat, die Einschränkungen von Java zu umgehen, die es nicht erlauben, Funktionen als erstklassige Bürger weiterzugeben. Wann immer Sie eine Funktion übergeben müssen, müssen Sie sie mit einer Methode wie der folgenden applyoder einer ähnlichen in eine Klasse einschließen.

Ich vermute, er hat nur eine Verknüpfung verwendet und direkt eine Klasse als Funktionsersatz verwendet. Wann immer Sie die Funktion ausführen möchten, instanziieren Sie einfach die Klasse.

Es ist definitiv kein gutes Muster, zumindest, weil wir hier versuchen, es herauszufinden, aber ich denke, es ist die am wenigsten ausführliche Methode, um etwas zu tun, das der funktionalen Programmierung in Java ähnelt.

Für mich ist die Faustregel, nichts im Konstruktor zu tun, außer Member-Variablen zu initialisieren. Der erste Grund dafür ist, dass es hilfreich ist, dem SRP-Prinzip zu folgen, da ein langer Initialisierungsprozess normalerweise ein Hinweis darauf ist, dass die Klasse mehr tut, als sie tun sollte, und die Initialisierung sollte außerhalb der Klasse erfolgen. Der zweite Grund ist, dass auf diese Weise nur die erforderlichen Parameter übergeben werden, sodass Sie weniger gekoppelten Code erstellen. Ein Konstruktor mit komplizierter Initialisierung benötigt normalerweise Parameter, die nur zum Erstellen eines anderen Objekts verwendet werden, von der ursprünglichen Klasse jedoch nicht verwendet werden.

Ich werde hier gegen die Flut vorgehen - in Sprachen, in denen alles in einer Klasse sein muss UND Sie keine statische Klasse haben können, können Sie in die Situation geraten, in der Sie ein isoliertes Stück Funktionalität haben, das sein muss setzt irgendwo und mit allem , was nicht sonst passen. Sie haben die Wahl zwischen einer Utility-Klasse, die verschiedene nicht zusammenhängende Funktionen abdeckt, oder einer Klasse, die im Grunde nur diese eine Sache tut.

Wenn Sie nur ein bisschen wie dieses haben, dann ist Ihre statische Klasse Ihre spezifische Klasse. Sie haben also entweder einen Konstruktor, der die ganze Arbeit erledigt, oder eine einzelne Funktion, die die ganze Arbeit erledigt. Der Vorteil, alles im Konstruktor zu tun, ist, dass es nur einmal vorkommt und Sie private Felder, Eigenschaften und Methoden verwenden können, ohne sich Gedanken darüber zu machen, dass diese wiederverwendet oder eingefädelt werden. Wenn Sie einen Konstruktor / eine Methode haben, könnten Sie versucht sein, die Parameter an die einzelne Methode zu übergeben, aber wenn Sie private Felder oder Eigenschaften verwenden, die potenzielle Threading-Probleme verursachen.

Selbst mit einer Utility-Klasse werden die meisten Leute nicht daran denken, geschachtelte statische Klassen zu haben (und das ist je nach Sprache möglicherweise auch nicht möglich).

Grundsätzlich ist dies eine relativ verständliche Methode, um das Verhalten vom Rest des Systems zu isolieren, innerhalb dessen, was die Sprache zulässt, und gleichzeitig die Sprache so gut wie möglich auszunutzen.

Ehrlich gesagt hätte ich viel geantwortet wie Ihr Auftragnehmer. Es ist eine geringfügige Änderung, es in zwei Anrufe umzuwandeln. Es gibt nicht so viel, was man übereinander empfehlen kann. Welche Dis- / Advatanges gibt es, sind wahrscheinlich eher hypothetisch als tatsächlich, warum sollte man versuchen, die Entscheidung zu rechtfertigen, wenn es keine Rolle spielt und es wahrscheinlich nicht so sehr entschieden wurde, als nur die Standardaktion?

Es gibt Muster, die in Sprachen üblich sind, in denen Funktionen und Klassen viel mehr dasselbe sind, wie in Python, wo man ein Objekt verwendet, um mit zu vielen Nebenwirkungen zu funktionieren, oder wenn mehrere benannte Objekte zurückgegeben werden.

In diesem Fall kann der Großteil der Arbeit, wenn nicht die gesamte Arbeit im Konstruktor ausgeführt werden, da das Objekt selbst nur ein Wrapper um ein einzelnes Arbeitsbündel ist und es keinen guten Grund gibt, es in eine separate Funktion zu packen. So etwas wie

var parser = XMLParser()
var x = parser.parse(string)
string a = x.LookupTag(s)

ist wirklich nicht besser als

 var x = ParsedXML(string)
 string a = x.LookupTag(s)

Anstatt die Nebenwirkungen direkt zu haben, können Sie das Objekt aufrufen, um die Nebenwirkung auf das Cue zu erzwingen, wodurch Sie eine bessere Sichtbarkeit erhalten. Wenn ich einen schlechten Nebeneffekt auf ein Objekt habe, kann ich meine ganze Arbeit tun und werde dann an dem Objekt vorbeigekommen, das ich schlecht auswirken werde, und füge ihm meinen Schaden zu. Das Identifizieren des Objekts und das Isolieren des Aufrufs auf diese Weise ist klarer als das gleichzeitige Übergeben mehrerer solcher Objekte an eine Funktion.

x.UpdateView(v)

Sie können die mehreren Rückgabewerte über Accessoren für das Objekt vornehmen. Die ganze Arbeit ist erledigt, aber ich möchte alles im Kontext haben und ich möchte nicht wirklich mehrere Referenzen an meine Funktion übergeben.

var x = new ComputeStatistics(inputFile)
int max = x.MaxOptions;
int mean = x.AverageOption;
int sd = x.StandardDeviation;
int k = x.Kurtosis;
...

Als gemeinsamer Python / C # -Programmierer erscheint mir das nicht sonderlich seltsam.

In einem Fall erledigt der Konstruktor / Destruktor die ganze Arbeit:

Schlösser. Normalerweise interagiert man während seines Lebens nie mit einem Schloss. Die Verwendung von C # -Architektur eignet sich für die Behandlung solcher Fälle, ohne explizit auf den Destruktor zu verweisen. Allerdings werden nicht alle Sperren auf diese Weise implementiert.

Ich habe auch geschrieben, was einem reinen Konstruktor-Code zum Patchen eines Laufzeitbibliotheksfehlers entsprach. (Das eigentliche Korrigieren des Codes hätte andere Probleme verursacht.) Es gab keinen Destruktor, da der Patch nicht rückgängig gemacht werden musste.

Ich stimme AndyBursh zu. Es scheint ein Mangel an Wissen zu geben.

Es ist jedoch wichtig, Frameworks wie NHibernate zu erwähnen, bei denen Sie nur im Konstruktor codieren müssen (wenn Sie Kartenklassen erstellen). Dies ist jedoch keine Einschränkung des Frameworks, sondern genau das, was Sie brauchen. In Bezug auf die Reflektion ist der Konstruktor die einzige Methode, die immer existiert (obwohl sie möglicherweise privat ist). Dies kann hilfreich sein, wenn Sie eine Methode einer Klasse dynamisch aufrufen müssen.

"Mir wurde vor langer Zeit beigebracht, dass instanziierte Objekte in einer Schleife im Allgemeinen eine schlechte Idee sind."

Ja, vielleicht erinnern Sie sich, warum wir den StringBuilder brauchen.

Es gibt zwei Java-Schulen.

Der Erbauer wurde vom ersten befördert , der uns lehrt, wiederzuverwenden (weil Teilen = Identität zur Leistungsfähigkeit). Anfang 2000 begann ich zu lesen, dass das Erstellen von Objekten in Java so billig ist (im Gegensatz zu C ++) und GC so effizient ist, dass die Wiederverwendung wertlos ist und nur die Produktivität des Programmierers von Bedeutung ist. Für die Produktivität muss er überschaubaren Code schreiben, was eine Modularisierung (auch bekannt als Einschränkung des Umfangs) bedeutet. Damit,

das ist schlecht

byte[] array = new byte[kilos or megas]
for_loop:
  use(array)

das ist gut.

for_loop:
  byte[] array = new byte[kilos or megas]
  use(array)

Ich habe diese Frage gestellt, als forum.java.sun existierte, und die Leute waren sich einig, dass sie es vorziehen würden, das Array so eng wie möglich zu machen.

Moderner Java-Programmierer zögert nie, eine neue Klasse zu deklarieren oder ein Objekt zu erstellen. Er wird dazu ermutigt. Java gibt allen Grund dazu mit seiner Idee, dass "alles ein Objekt ist" und billige Schöpfung.

Darüber hinaus besteht ein moderner Programmierstil für Unternehmen darin, wenn Sie eine Aktion ausführen müssen, eine spezielle Klasse (oder noch besser ein Framework) zu erstellen, die diese einfache Aktion ausführt. Gute Java IDEs, die hier helfen. Sie erzeugen automatisch Tonnen von Mist als Atmung.

Ich denke also, Ihren Objekten fehlt das Builder- oder Factory-Muster. Es ist nicht anständig, Instanzen direkt vom Konstruktor zu erstellen. Für jedes Objekt wird eine spezielle Werksklasse empfohlen.

Wenn nun die funktionale Programmierung ins Spiel kommt, wird das Erstellen von Objekten noch einfacher. Sie erstellen Objekte bei jedem Schritt als Atmung, ohne es zu merken. Ich gehe davon aus, dass Ihr Code in einer neuen Ära noch "effizienter" wird

"Tun Sie nichts in Ihrem Konstruktor. Es ist nur für die Initialisierung"

Persönlich sehe ich keinen Grund in der Richtlinie. Ich halte es nur für eine andere Methode. Das Ausklammern von Code ist nur erforderlich, wenn Sie ihn freigeben können.