2010-12-21 19 views
10

Bei extrem kurzlebigen Objekten, für die ich nur eine Methode aufrufen muss, bin ich geneigt, den Methodenaufruf direkt an new zu ketten. Ein sehr bekanntes Beispiel hierfür ist etwa wie folgt:Gibt es einen Grund, `new object(). Foo()` nicht zu verwenden?

string noNewlines = new Regex("\\n+").Replace(" ", oldString); 

Der Punkt hier ist, dass ich keine Notwendigkeit für das Regex Objekt habe, nachdem ich den einen Ersatz gemacht habe, und Ich mag diese Lage sein, auszudrücken als One-Liner. Gibt es ein nicht offensichtliches Problem mit diesem Idiom? Einige meiner Mitarbeiter haben sich damit unwohl gefühlt, aber ohne etwas, das wie ein guter Grund zu sein schien.

(Ich habe dies als C# und Java markiert, da das oben Idiom in beiden Sprachen verbreitet und nutzbar ist.)

+4

Nur würde ich dies nicht tun, wenn das 'neue Objekt()' 'IDisposable' implementiert. Offensichtlich sieht 'Regex' mir nicht so gut aus. – SwDevMan81

+0

@ SwDevMan81 - Sollte das als Antwort haben! – Reddog

+0

@Reddog - Sollte haben :) – SwDevMan81

Antwort

6

Dieses besondere Muster ist in Ordnung - ich benutze es mich gelegentlich.

Aber ich würde dieses Muster nicht verwenden, wie Sie in Ihrem Beispiel haben. Es gibt zwei alternative Ansätze, die besser sind.

Besserer Ansatz: Verwenden Sie die statische Methode Regex.Replace(string,string,string). Es gibt keinen Grund, Ihre Bedeutung mit der new-Syntax zu verschleiern, wenn eine statische Methode verfügbar ist, die dasselbe tut.

beste Ansatz: Wenn Sie die gleiche statische (nicht dynamisch generierte) Regex aus der gleichen Methode zu verwenden, und Sie diese Methode viel anrufen, sollten Sie speichern die Regex-Objekt als privates statisches Feld für die Klasse enthält, die Methode, da dies vermeidet, den Ausdruck bei jedem Aufruf der Methode zu analysieren.

+0

Irgendwie übersah ich die Tatsache, dass es eine statische Methode gibt, die genau das tut. Was Ihren zweiten Punkt betrifft, ist dies eine selten genannte Funktion, sonst hätte ich die Regex in eine Membervariable verschoben, um sie nicht wiederholt zu erstellen. –

5

Ich sehe nichts falsch damit; Ich mache das ziemlich oft selbst.

Die einzige Ausnahme von der Regel könnte für Debugging-Zwecke sein, manchmal ist es notwendig, den Zustand des Objekts im Debugger zu sehen, was in einem solchen Einliner schwierig sein kann.

+0

+1 Guter Punkt__ – SwDevMan81

1

Solange Sie sicher sind, dass das Objekt nie wieder benötigt wird (oder Sie nicht mehrere Instanzen eines identischen Objekts erstellen), gibt es kein Problem damit.

Wenn der Rest Ihres Teams sich nicht damit wohl fühlt, sollten Sie die Entscheidung überdenken. Das Team sollte Standards setzen und Sie sollten ihnen folgen. Sei konsistent. Wenn Sie den Standard ändern möchten, besprechen Sie es. Wenn sie nicht einverstanden sind, dann passen Sie sich an.

1

Ich denke, das ist in Ordnung, und würde Kommentare/Gründe für das Gegenteil begrüßen. Wenn das Objekt nicht kurzlebig ist (oder nicht verwaltete Ressourcen verwendet - zB COM), kann diese Vorgehensweise Sie in Schwierigkeiten bringen.

2

Wenn Sie das Objekt danach nicht brauchen, sehe ich kein Problem - ich mache es auch von Zeit zu Zeit. Es kann jedoch sehr schwierig sein, sie zu erkennen. Wenn Ihre Mitarbeiter also Unbehagen ausdrücken, müssen Sie sie vielleicht in eine Variable schreiben, damit Sie keine schweren Gefühle im Team haben. Tut dir nicht wirklich weh.

2

Sie müssen nur vorsichtig sein, wenn Sie Methoden von Objekten verketten, die IDisposable implementieren. Bei einer einzeiligen Kette bleibt kein Platz für den Aufruf von Dispose oder den Block {...}.

Zum Beispiel:

DialogResult result = New SomeCfgDialog(some_data).ShowDialog(); 

Es gibt keine Instanz-Variable, auf das Entsorgen zu nennen.

Dann gibt es Potenzial, Absicht zu verschleiern, zu verletzen, anstatt die Lesbarkeit zu verbessern und es schwieriger zu machen, Werte während des Debuggens zu untersuchen. Aber das sind alles Themen, die sich auf das Objekt und die Situation und die Anzahl der verketteten Methoden beziehen. Ich glaube nicht, dass es einen einzigen Grund gibt, es zu vermeiden. Manchmal macht dies den Code prägnanter und lesbarer und manchmal kann es aus einigen der oben genannten Gründe schmerzen.

0

Das einzige mögliche Problem, das ich sehen konnte, ist - wenn aus irgendeinem Grund, neue Regex NULL waren, weil es nicht instanziiert wurde, würden Sie eine Null-Zeiger-Ausnahme erhalten. Allerdings bezweifle ich stark, dass seit Regex immer definiert ist ...

+1

Die einzige Möglichkeit, wie ein Konstruktor fehlschlagen kann, ist das Auslösen einer Ausnahme, die nach oben weitergegeben wird und die Ausführung unterbricht. Ein Konstruktor wird * niemals * null zurückgeben. (Konstruktoren geben eigentlich nichts zurück ...) – cdhowie

+1

'neues X (...)' kann niemals zu 'null' auswerten - es kann eine Exception auslösen.Sie haben jedoch einen Punkt darin, dass ein 'null' * irgendwo * (else) in einem" verketteten "Ausdruck zu einem schwer zu findenden NPE führen kann (da der Stack-Trace nur auf die Linie zeigt, nicht auf den Fehler) Ausdruck): 'foo(). returnnullstimes(). bar() // aww, crum ' –

1

Das Problem ist Lesbarkeit.

Die "verketteten" Methoden in einer separaten Zeile zu platzieren scheint die bevorzugte Vereinbarung mit meinem Team zu sein.

string noNewlines = new Regex("\\n+") 
          .Replace(" ", oldString); 
+0

+1 für die Lesbarkeit. –

1

Ein Grund, diesen Stil zu vermeiden, ist, dass Ihre Mitarbeiter das Objekt möglicherweise im Debug-Modus untersuchen möchten. Wenn Sie die ähnliche Instantiierung zusammensetzen, sinkt die Lesbarkeit sehr. Zum Beispiel:

String val = new Object1("Hello").doSomething(new Object2("interesting").withThis("input")); 

Im Allgemeinen bevorzuge ich eine statische Methode für das spezifische Beispiel, das Sie erwähnt haben.

0

Wenn Sie sich nicht für das Objekt interessieren, auf das Sie die Methode anwenden, ist das ein Zeichen, dass die Methode wahrscheinlich statisch sein sollte.

0

In C#, würde ich wahrscheinlich eine Erweiterungsmethode schreiben die Regex zu wickeln, so dass ich

string noNewlines = oldString.RemoveNewlines(); 

Die Erweiterungsmethode wie

etwas aussehen würde schreiben konnte
using System.Text.RegularExpressions; 

namespace Extensions 
{ 
    static class SystemStringExtensions 
    { 
     public static string RemoveNewlines(this string inputString) 
     { 
      // replace newline characters with spaces 
      return Regex.Replace(inputString, "\\n+", " "); 
     } 
    } 
} 

ich das finden viel einfacher zu lesen als das ursprüngliche Beispiel. Es ist auch ziemlich wiederverwendbar, da das Entfernen von Newline-Zeichen eine der häufigsten Aktivitäten ist.

Verwandte Themen