Czy logowanie obok implementacji stanowi naruszenie zasad SRP?

19

Myśląc o zwinnym tworzeniu oprogramowania i wszystkich zasadach (SRP, OCP, ...) zadaję sobie pytanie, jak traktować rejestrowanie.

Czy logowanie obok implementacji stanowi naruszenie zasad SRP?

Powiedziałbym, yesponieważ wdrożenie powinno być również w stanie działać bez logowania. Jak więc lepiej wdrożyć rejestrowanie? Sprawdziłem niektóre wzory i doszedłem do wniosku, że najlepszym sposobem, aby nie naruszać zasad w sposób zdefiniowany przez użytkownika, ale użyć dowolnego wzoru, o którym wiadomo, że narusza zasadę, jest użycie wzoru dekoratora.

Załóżmy, że mamy kilka komponentów całkowicie bez naruszenia SRP, a następnie chcemy dodać rejestrowanie.

  • składnik A
  • składnik B wykorzystuje A

Chcemy logowania dla A, więc tworzymy kolejny komponent D ozdobiony A, oba implementujące interfejs I.

  • interfejs I
  • komponent L (komponent rejestrujący systemu)
  • składnik A implementuje I
  • składnik D implementuje I, ozdabia / używa A, używa L do logowania
  • składnik B wykorzystuje I

Zalety: - Mogę używać A bez logowania - testowanie A oznacza, że ​​nie potrzebuję żadnych próbnych rejestrów - testy są prostsze

Wada: - więcej komponentów i więcej testów

Wiem, że to wydaje się być kolejnym otwartym pytaniem do dyskusji, ale tak naprawdę chcę wiedzieć, czy ktoś stosuje lepsze strategie rejestrowania niż naruszenie dekoratora lub SRP. Co z statycznym logowaniem singleton, które są domyślnie NullLogger, a jeśli potrzebne jest rejestrowanie syslog, należy zmienić obiekt implementacji w czasie wykonywania?

H
źródło
Już to przeczytałem i przepraszam, odpowiedź nie jest satysfakcjonująca.
Aitch
@ MarkRogers dziękuję za udostępnienie tego interesującego artykułu. Wujek Bob mówi w „Clean Code”, że ładny komponent SRP ma do czynienia z innymi komponentami na tym samym poziomie abstrakcji. Dla mnie to wyjaśnienie jest łatwiejsze do zrozumienia, ponieważ kontekst może być zbyt duży. Ale nie mogę odpowiedzieć na pytanie, ponieważ jaki jest kontekst lub abstrakcja poziomu rejestratora?
Aitch
3
„nie jest dla mnie odpowiedzią” lub „odpowiedź nie jest satysfakcjonująca” jest nieco lekceważące. Możesz zastanowić się, co konkretnie jest niezadowalające (jakie masz wymagania, które nie zostały spełnione przez tę odpowiedź? Co konkretnie jest wyjątkowe w twoim pytaniu?), A następnie edytuj pytanie, aby upewnić się, że to wymaganie / unikalny aspekt jest wyjaśnione w jasny sposób. Celem jest, abyś edytował swoje pytanie, aby je ulepszyć, aby było jaśniejsze i bardziej skoncentrowane, a nie prosić o potwierdzenie, że twoje pytanie jest inne / nie powinno być zamykane bez uzasadnienia. (Możesz także skomentować drugą odpowiedź.)
DW

Odpowiedzi:

-1

Tak, jest to naruszenie SRP, ponieważ rejestracja jest zagadnieniem przekrojowym.

Prawidłowym sposobem jest przekazanie logowania do klasy rejestratora (przechwytywanie), której jedynym celem jest rejestrowanie - zgodnie z SRP.

Zobacz ten link jako dobry przykład: https://msdn.microsoft.com/en-us/library/dn178467%28v=pandp.30%29.aspx

Oto krótki przykład :

public interface ITenantStore
{
    Tenant GetTenant(string tenant);
    void SaveTenant(Tenant tenant);
}

public class TenantStore : ITenantStore
{
    public Tenant GetTenant(string tenant)
    {....}

    public void SaveTenant(Tenant tenant)
    {....}
} 

public class TenantStoreLogger : ITenantStore
{
    private readonly ILogger _logger; //dep inj
    private readonly ITenantStore _tenantStore;

    public TenantStoreLogger(ITenantStore tenantStore)
    {
        _tenantStore = tenantStore;
    }

    public Tenant GetTenant(string tenant)
    {
        _logger.Log("reading tenant " + tenant.id);
        return _tenantStore.GetTenant(tenant);
    }

    public void SaveTenant(Tenant tenant)
    {
        _tenantStore.SaveTenant(tenant);
        _logger.Log("saving tenant " + tenant.id);
    }
}

Korzyści obejmują

  • Możesz to przetestować bez logowania - prawdziwe testowanie jednostkowe
  • możesz łatwo włączać / wyłączać logowanie - nawet w czasie wykonywania
  • możesz zastąpić rejestrowanie innymi formami rejestrowania bez konieczności zmiany pliku TenantStore.
z0mbi3
źródło
Dziękuję za miły link. Rysunek 1 na tej stronie to tak naprawdę to, co nazwałbym moim ulubionym rozwiązaniem. Lista zagadnień przekrojowych (rejestrowanie, buforowanie itp.) I wzór dekoratora jest najbardziej ogólnym rozwiązaniem i cieszę się, że nie mylę się całkowicie z moimi myślami, chociaż większa społeczność chciałaby porzucić tę abstrakcję i rejestrować bezpośrednio .
Aitch,
2
Nigdzie nie widzę, jak przypisujesz zmienną _logger. Czy planowałeś użyć iniekcji konstruktora i po prostu zapomniałeś? Jeśli tak, prawdopodobnie otrzymasz ostrzeżenie kompilatora.
user2023861,
27
Zamiast TenantStore jest zamaczany za pomocą Loggera ogólnego przeznaczenia, który wymaga klas N + 1 (gdy dodajesz LandlordStore, FooStore, BarStore itp.), Masz TenantStoreLogger jest zamaczany za pomocą TenantStore, a FooStoreLogger jest zamaczany za pomocą FooStore itp.… wymagające klas 2N. O ile mogę powiedzieć, dla zerowej korzyści. Jeśli chcesz przeprowadzić testowanie bez logowania, musisz ponownie uruchomić klasy N, a nie tylko skonfigurować NullLogger. IMO, to bardzo złe podejście.
user949300,
6
Wykonanie tego dla każdej klasy, która wymaga rejestrowania, drastycznie zwiększa złożoność bazy kodu (chyba że tak niewiele klas ma rejestrowanie, że nawet nie nazwałbyś tego problemem przekrojowym). To ostatecznie sprawia, że ​​kod jest trudniejszy do utrzymania po prostu z powodu dużej liczby obsługiwanych interfejsów, co jest sprzeczne ze wszystkim, dla czego stworzono zasadę pojedynczej odpowiedzialności.
jpmc26
9
Doceniony. Usunąłeś problem z rejestrowaniem z klasy Najemca, ale teraz TenantStoreLoggerzmienisz się za każdym razem, gdy się TenantStorezmieni. Nie rozdzielasz problemów bardziej niż w pierwotnym rozwiązaniu.
Laurent LA RIZZA
61

Powiedziałbym, że zbyt poważnie podchodzisz do SRP. Jeśli Twój kod jest wystarczająco uporządkowany, aby logowanie było jedynym „naruszeniem” SRP, radzisz sobie lepiej niż 99% wszystkich innych programistów i powinieneś poklepać się po plecach.

Celem SRP jest unikanie przerażającego kodu spaghetti, w którym kod, który robi różne rzeczy, jest pomieszany. Mieszanie rejestrowania z kodem funkcjonalnym nie wywołuje dla mnie żadnych dzwonków alarmowych.

grahamparki
źródło
19
@Aitch: Masz do wyboru sztywne połączenie logowania do klasy, przekazanie uchwytu do programu rejestrującego lub nic nie logować. Jeśli zamierzasz być bardzo rygorystyczny w stosunku do SRP kosztem wszystkiego innego, radzę nigdy nie rejestrować niczego. Wszystko, co musisz wiedzieć o tym, co robi twoje oprogramowanie, można wypróbować za pomocą debugera. P w SRP oznacza „zasadę”, a nie „fizyczne prawo natury, którego nigdy nie wolno łamać”.
Blrfl,
3
@Aitch: Powinieneś być w stanie prześledzić rejestrowanie w swojej klasie z powrotem do pewnych wymagań, w przeciwnym razie naruszasz YAGNI. Jeśli rejestrowanie jest na stole, podajesz prawidłowy uchwyt rejestratora, tak jak dla wszystkich innych potrzebnych klas, najlepiej tej z klasy, która już przeszła testy. To, czy tworzy rzeczywiste wpisy do dziennika, czy zrzuca je do segmentu bitów, zależy od tego, co utworzyło instancję klasy; sama klasa nie powinna się tym przejmować.
Blrfl,
3
@Aitch Aby odpowiedzieć na pytanie dotyczące testowania jednostkowego Do you mock the logger?:, to jest PRECYZYJNIE to, co robisz. Powinieneś mieć ILoggerinterfejs, który określa CO robi rejestrator. Testowany kod jest wstrzykiwany z określonym ILoggerprzez Ciebie. Do testowania masz class TestLogger : ILogger. Wspaniałą rzeczą w tym jest to, że TestLoggermoże ujawniać rzeczy takie jak ostatni zarejestrowany ciąg lub błąd. Testy mogą sprawdzić, czy testowany kod loguje się poprawnie. Na przykład testem może być UserSignInTimeGetsLogged()test sprawdzania TestLoggerzalogowanego.
CurtisHx,
5
99% wydaje się nieco niski. Prawdopodobnie jesteś lepszy niż 100% wszystkich programistów.
Paul Draper,
2
+1 za zdrowie psychiczne. Potrzebujemy więcej tego rodzaju myślenia: mniejszej koncentracji na słowach i abstrakcyjnych zasadach, a bardziej na utrzymywaniu bazy kodu, którą można utrzymać .
jpmc26
15

Nie, to nie jest naruszenie SRP.

Wiadomości wysyłane do dziennika powinny ulec zmianie z tych samych powodów, co otaczający kod.

Czym jest naruszenie SRP, polega na użyciu określonej biblioteki do logowania bezpośrednio w kodzie. Jeśli zdecydujesz się zmienić sposób rejestrowania, SRP stwierdza, że ​​nie powinno to wpływać na kod biznesowy.

LoggerKod abstrakcyjny powinien być dostępny dla pewnego rodzaju streszczenia , a jedyną rzeczą, którą powinna powiedzieć implementacja, jest „Wyślij tę wiadomość do dziennika”, bez obaw o sposób jej wykonania. Decydowanie o dokładnym sposobie rejestrowania (nawet znaczników czasu) nie jest obowiązkiem twojej implementacji.

Twoja implementacja nie powinna również wiedzieć, czy program rejestrujący, do którego wysyła wiadomości, jest NullLogger.

To mówi.

Nie odrzuciłbym zbyt szybko wylogowywania się jako problemu przekrojowego . Emitowanie dzienników w celu śledzenia określonych zdarzeń występujących w kodzie implementacji należy do kodu implementacji.

OTOH jest zagadnieniem przekrojowym. Śledzenie wykonania : rejestrowanie wchodzi i wychodzi w każdej metodzie. AOP jest najlepiej przygotowany do tego.

Laurent LA RIZZA
źródło
Powiedzmy, że komunikat rejestratora to „login user xyz”, który jest wysyłany do rejestratora poprzedzającego znacznik czasu itp. Czy wiesz, co oznacza „logowanie” dla implementacji? Czy rozpoczyna się sesja z plikiem cookie lub innym mechanizmem? Myślę, że istnieje wiele różnych sposobów implementacji logowania, dlatego zmiana implementacji nie ma logicznego związku z faktem, że użytkownik się loguje. To kolejny świetny przykład dekorowania różnych komponentów (np. OAuthLogin, SessionLogin, BasicAuthorizationLogin) robiąc to samo jako Logininterfejs ozdobiony tym samym rejestratorem.
Aitch
Zależy, co oznacza komunikat „zaloguj się użytkownik xyz”. Jeśli oznacza to, że logowanie się powiodło, wysłanie wiadomości do dziennika należy do przypadku użycia logowania. Konkretny sposób przedstawienia informacji logowania jako ciągu (OAuth, Session, LDAP, NTLM, odcisk palca, koło chomika) należy do określonej klasy reprezentującej poświadczenia lub strategię logowania. Nie ma przekonującej potrzeby, aby go usunąć. Ten szczególny przypadek nie jest przekrojowy. Jest to specyficzne dla przypadku użycia logowania.
Laurent LA RIZZA
7

Ponieważ rejestrowanie jest często uważane za problem przekrojowy, sugeruję użycie AOP do oddzielenia rejestrowania od implementacji.

W zależności od języka użyjesz do tego celu przechwytywacza lub jakiegoś środowiska AOP (np. AspectJ w Javie).

Pytanie brzmi, czy to naprawdę jest kłopotliwe. Pamiętaj, że ta separacja zwiększy złożoność projektu, zapewniając jednocześnie bardzo niewielkie korzyści.

Oliver Weiler
źródło
2
Większość kodu AOP, który widziałem, dotyczyła rejestrowania każdego kroku wejścia i wyjścia każdej metody. Chcę tylko zarejestrować niektóre części logiki biznesowej. Może więc możliwe jest rejestrowanie tylko metod z adnotacjami, ale AOP w ogóle może istnieć tylko w językach skryptowych i środowiskach maszyn wirtualnych, prawda? W np. C ++ jest to niemożliwe. Przyznaję, że nie jestem bardzo zadowolony z podejść AOP, ale może nie ma czystszego rozwiązania.
Aitch
1
@H. „C ++ to niemożliwe”. : Jeśli google dla „aop c ++” znajdziesz artykuły na ten temat. „... kod AOP, który widziałem, dotyczył rejestrowania każdego kroku wejścia i wyjścia każdej metody. Chcę tylko zalogować niektóre elementy logiki biznesowej.” Aop pozwala definiować wzorce w celu znalezienia metod modyfikacji. tzn. wszystkie metody z przestrzeni nazw „my.busininess. *”
k3b
1
Rejestrowanie często NIE stanowi problemu przekrojowego, szczególnie gdy chcesz, aby dziennik zawierał interesujące informacje, tj. Więcej informacji niż zawiera ślad stosu wyjątku.
Laurent LA RIZZA
5

Brzmi nieźle. Opisujesz dość standardowy dekorator drewna. Ty masz:

komponent L (komponent rejestrujący systemu)

Na tym spoczywa jedna odpowiedzialność: rejestrowanie przekazywanych mu informacji.

składnik A implementuje I

Ma to jeden obowiązek: zapewnienie implementacji interfejsu I (przy założeniu, że jestem odpowiednio zgodny z SRP).

To jest kluczowa część:

składnik D implementuje I, ozdabia / używa A, używa L do logowania

Mówiąc w ten sposób, brzmi to skomplikowanie, ale spójrz na to w ten sposób: Komponent D robi jedną rzecz: łączy A i L.

  • Składnik D nie rejestruje się; przekazuje to L
  • Komponent D nie implementuje samego I; przekazuje to do A.

Tylko odpowiedzialność składnik D ma to, aby upewnić się, że L jest powiadamiany, gdy stosuje się. Implementacje A i L znajdują się gdzie indziej. Jest to całkowicie zgodne z SRP, a także jest dobrym przykładem OCP i dość powszechnym zastosowaniem dekoratorów.

Ważne zastrzeżenie: gdy D używa komponentu rejestrującego L, powinno to robić w sposób umożliwiający zmianę sposobu rejestrowania. Najprostszym sposobem na to jest posiadanie interfejsu IL, który jest implementowany przez L. Następnie:

  • Składnik D używa IL do logowania; instancja L jest zapewniona
  • Komponent D używa I w celu zapewnienia funkcjonalności; zapewniona jest instancja A.
  • Komponent B wykorzystuje I; zapewniona jest instancja D.

W ten sposób nic nie zależy bezpośrednio od niczego innego, co ułatwia ich wymianę. Ułatwia to dostosowanie się do zmian i kpiny z części systemu, dzięki czemu można przeprowadzić test jednostkowy.

anaksymander
źródło
Właściwie znam tylko C #, który ma natywną obsługę delegacji. Dlatego napisałem D implements I. Dziękuję za Twoją odpowiedź.
Aitch,
1

Oczywiście jest to naruszenie SRP, ponieważ masz problem przekrojowy. Możesz jednak utworzyć klasę odpowiedzialną za tworzenie rejestrowania przy wykonywaniu dowolnej akcji.

przykład:

class Logger {
   ActuallLogger logger;
   public Action ComposeLog(string msg, Action action) {
      return () => {
          logger.debug(msg);
          action();
      };
   }
}
Paweł Nikonowicz
źródło
2
Doceniony. Wyręb jest rzeczywiście problemem przekrojowym. Podobnie jak wywołania metod sekwencjonowania w kodzie. To nie wystarczający powód, by twierdzić, że naruszono SRP. Rejestrowanie wystąpienia określonego zdarzenia w aplikacji NIE jest zagadnieniem przekrojowym. Sposób, w jaki te wiadomości są przekazywane do każdego zainteresowanego użytkownika, jest rzeczywiście osobnym problemem, a opisanie tego w kodzie implementacyjnym JEST naruszeniem SRP.
Laurent LA RIZZA
„wywołania metod sekwencjonowania” lub kompozycja funkcjonalna nie są zagadnieniem przekrojowym, ale raczej szczegółem implementacji. Zadaniem utworzonej przeze mnie funkcji jest utworzenie instrukcji dziennika z działaniem. Nie muszę używać słów „i”, aby opisać działanie tej funkcji.
Paul Nikonowicz,
To nie jest szczegół implementacji. Ma to ogromny wpływ na kształt twojego kodu.
Laurent LA RIZZA
Myślę, że patrzę na SRP z perspektywy „CO robi ta funkcja”, gdy patrzysz na SRP z perspektywy „JAK robi to funkcja”.
Paul Nikonowicz