2016-11-30 22 views
8

Nie podoba mi się sposób, w jaki zaprojektowałem prosty program. Istnieje obiekt FileParser, który ma zdarzenia OnFileOpened, OnLineParsed i OnFileClosed oraz kilka obiektów, które tworzą mniejsze pliki w oparciu o zawartość pliku, który jest analizowany przez FileParser.Idiomatyczne C# do interakcji obiektu tylko po zarejestrowaniu na zdarzenia

Obiekty obserwujące FileParser rejestrują swoje metody ze zdarzeniami FileParser w swoich konstruktorach.

ParsedFileFrobber(FileParser fileParser) 
{ 
    fileParser.OnFileOpen += this.OpenNewFrobFile; 
    fileParser.OnLineParsed += this.WriteFrobbedLine; 
    fileParser.OnFileClose += this.CloseFrobFile; 
} 

Po tym, ParsedFileFrobber prostu wsiada z rzeczy bez dalszej wyraźnej interakcji.

Rzeczą, która mnie martwi jest to, że główny program przypisuje tylko ParsedFileFrobber i zasadniczo nie używa wartości zwracanej przez konstruktora, by tak rzec.

var fileParser = new FileParser(myFilename); 
var parsedFileFrobber = new ParsedFileFrobber(fileParser); 
// No further mentions of parsedFileFrobber. 

Działa, ale ReSharper narzeka na to, co przynajmniej zmusza mnie do zastanowienia. Właściwie, nie potrzebuję nawet przypisać zmiennej do wyniku konstruktora, ponieważ GC będzie utrzymywał przy życiu ParsedFileFrobber dzięki referencjom obsługi zdarzenia, ale goły new wygląda mi bardzo źle. (Ciągle kompiluje i działa poprawnie.)

var fileParser = new FileParser(myFilename); 
new ParsedFileFrobber(fileParser); 

Czy to jest problem? Czy jest to anty-wzór lub zapach kodu? Czy istnieje już idiomatyczny sposób robienia tego w języku C#?

Dzięki!

Wyjaśnienia dzięki pomocne komentarze:

1) Dlaczego nie odwrócić związek? Cody Gray

Ah, przykład był nieco zbyt uproszczony. W rzeczywistości mam ParsedFileFrobber, ParsedFileGrobber i ParsedFileBrobber. Odwrócenie relacji uzależniłoby wszystkie 3. (Początkowo był tylko Frobber i Grobber, ale później było zapotrzebowanie na Brobbera, a wciąż istnieje możliwość, aby Drobber i tak dalej. Wydaje mi się, że to kwestia gustu w kwestii tego, czy linie kodu wykonującego subskrypcję występują w konstruktorze FileParser, czy w konstruktorach ParsedFileFrobber, ParsedFileGrobber i ParsedFileBrobber, ale moim preferowaniem jest próba zachowania wartości parametru FileParser jako agnostycznej.

2) Dlaczego nie przenieść konstruktora do metody statycznej (i uczynić konstruktorem prywatnym)? Hans Passant

Widzę, jak te odstraszają potencjalnie nieintuicyjne użycie w prywatnej pracy klasy, co jest dobrą radą. Jednak nadal będzie to albo gołe new, albo jedyne w historii odniesienie do zwracanej wartości konstruktora. Cóż, jeśli to nie jest duży problem, warto ukryć brzydki kod. (Dla porównania, zrobiłem dokonać ParsedFileFrobberIDisposable z metodą Dispose wypisywania ze zdarzeń, jest więc możliwe, aby położyć kres frobbing.)

Dziękuję wszystkim komentujących!

+2

Nie jestem całkowicie pewien, że rozumiem projekt, ale moje pierwsze pytanie byłoby, dlaczego nie odwrócić relacji? Obecnie program ParsedFileFrobber jest zależny od FileParser, ale nigdy nie użyje ponownie instancji ParsedFileFrobber. Dlaczego zamiast tego uczynić FileParser zależnym od ParsedFileFrobber, skoro jesteś (podobno) zachowując odwołanie do obiektu FileParser? –

+2

Nie ma się czym przejmować, tylko dobrym przypomnieniem, że będzie trwało wiecznie, dopóki obiekt FileParser pozostanie przy życiu. Możesz sprawić, by wyglądał "naturalny" przez dodanie statycznej metody do ParseFileFrobber. Nazwij go, erm, Frob() lub Observe(). Ustaw konstruktora jako prywatny. –

+0

Zastanawiam się, dlaczego nie będziesz potrzebować utworzonej instancji 'ParsedFileFrobber'. Oczywiście ta klasa (i jej krewni 'ParsedFile *') robią coś w programach obsługi zdarzeń wytwarzających pewien stan. Gdzie jest przechowywane to państwo? Z tego, co wiemy, musi to być wewnątrz 'ParsedFileFrobber'. A jeśli jesteśmy zainteresowani tym stanem (myślę, że jesteśmy, w przeciwnym razie, dlaczego mielibyśmy analizować plik mimo wszystko), musielibyśmy również uzyskać dostęp do instancji 'ParsedFileFrobber'. Wydaje mi się, że widzimy tylko część obrazu, a nie całą zawartość. –

Odpowiedz

0

W końcu poszedłem z czymś takim, jak zalecił Hans Passant: uprzątnięcie go w wewnętrzny sposób statycznej metody.

1

Jednym z możliwych rozwiązań może być użycie Mediator pattern, która wprowadza nową klasę, która kontroluje interakcję pomiędzy klasami FileParser i ParsedFile* :. W ten sposób Twoje zajęcia są bardzo luźno powiązane, Twój FileParser nie musi wiedzieć o ParsedFileFrobber i odwrotnie. Klasa mediatora może również zadbać o czas życia obiektów.

public interface IParsedFile 
{ 
    void OnOpen(); 
    void OnLineParsed(string line); 
    void OnClosed(); 
} 

public class FileParseManager 
{ 
    private readonly FileParser _fileParser; 
    private readonly List<IParsedFile> _parsedFiles; 

    public FileParseManager(FileParser fileParser, List<IParsedFile> parsedFiles) 
    { 
     _fileParser = fileParser; 
     _parsedFiles = parsedFiles; 
    } 

    public void Parse(string fileName) 
    { 
     _fileParser.OpenFile(string fileName); 
     foreach (var parsedFile in _parsedFiles) 
     { 
      parsedFile.OnOpen(); 
     } 

     while ((string line = _fileParser.GetNextLine()) != null) 
     { 
      foreach (var parsedFile in _parsedFiles) 
      { 
       parsedFile.OnLineParsed(line); 
      } 
     } 

     _fileParser.CloseFile(); 
     foreach (var parsedFile in _parsedFiles) 
     { 
      parsedFile.OnClosed(); 
     } 
    } 
} 

Innym rozwiązaniem może być użycie producent konsumentów wzór. Dostarczono coś takiego, np. przez TPL (Task Parallel Library) w .NET Framework. Spójrz na Dataflow section i przykłady tam dostarczone.

+0

Doceniam referencje, ale wydaje się, że odtwarzają one wyraźnie działanie systemu obsługi zdarzeń bez żadnych oczywistych korzyści, zwłaszcza że nie jest konieczne, aby Grobber miał na przykład OnOpen(), podczas gdy tutaj miałby do wdrożenia (jako no-op). –

Powiązane problemy