Wzorzec repozytorium. Kilka niepewności. Co zamiast

Czytam różne rzeczy na temat tego, jak wzorzec repozytorium powinien wyglądać.

Np. powinien posiadać interfejs generyczny: IRepository<TEntity>
Potem Generyczną klasę implementującą ten interfejs: Repository<TEntity>: IRepository<TEntity>
No i Konkretne klasy, np: ClientRepository: Repository<Client>

Inni mówią, że nie wzorzec nie musi być generyczny.
Kiedy lepiej stosować generyczny, a kiedy nie? Zaraz ktoś się pewnie przyczepi, że lepiej w ogóle nie stosować repozytorium. OK. Ale co zamiast? Do tej pory radziłem sobie z tym problemem nieco inaczej, ale chciałbym spróbować jakiegoś nowego podejścia. Nie używam żadnego ORMa.

Konkretne repozytoria mogą mieć wspólne bazowe repozytorium generyczne, które realizuje wspólne zadania. Jest to dobre podejście gdy repozytorium jest wraperem na ORMa, a ten posiada generyczne API. Jeśli repozytorium zawiera jakieś ręczne budowanie zapytań, to niekoniecznie jest sens wydzielać coś bazowego, a zwłaszcza generycznego.
Błędem jest wypuszczanie repozytorium generycznego do logiki biznesowej, bo ideą repozytorium jest bycie specjalizowaną kolekcją. A generyczny jest przeciwieństwem specjalizowanego.

Ogólnie czego nie zrobisz, to i tak na 95% nie wyjdzie Ci repozytorium tylko zwykłe DAO.

somekind napisał(a):

Konkretne repozytoria mogą mieć wspólne bazowe repozytorium generyczne, które realizuje wspólne zadania. Jest to dobre podejście gdy repozytorium jest wraperem na ORMa, a ten posiada generyczne API.

Czyli coś takiego:

public class Repository<TEntity>
{
    public void Add(TEntity ent);
    public void Delete(TEntity ent);
    public void Update(TEntity ent);
    public IEnumerable<TEntity> GetAll();
}

public class ClientRepository: Repository<Client>
{
    public IEnumerable<Client> GetClientsWithBigPenis();
}

zgadza się?

Jeśli repozytorium zawiera jakieś ręczne budowanie zapytań, to niekoniecznie jest sens wydzielać coś bazowego, a zwłaszcza >generycznego.

Ale czemu nie? Przecież w repozytorium mam jakiś data kontekst, który może budować pytania dynamicznie za pomocą np. jakiś atrybutów. Coś w deseń:

public class DataContext<TEntity>
{
  public void AddEntity(TEntity ent)
  {
    string tableName = ent.GetType().GetAttributeValue("TableName"); //pseudokod
  }
}

Czyli w klasie Client muszę mieć tylko atrybut TableName (nie wiem, czy to dobre). A nazwami pól będą po prostu nazwy właściwości. Tak chyba działają ORMy, nie?

Błędem jest wypuszczanie repozytorium generycznego do logiki biznesowej, bo ideą repozytorium jest bycie specjalizowaną kolekcją. A generyczny jest przeciwieństwem specjalizowanego.

Ogólnie czego nie zrobisz, to i tak na 95% nie wyjdzie Ci repozytorium tylko zwykłe DAO.

A czy repozytorium nie jest takim przykładem DAO?

W sumie to ja nie wiem czym się różni DAO od Repository
Według mnie to całkiem sensownie jest zrobione w Springu Data JPA gdzie jest generyczny interface podstawowy który jest w locie implementowany przez framework i podstawowe operacje są "współdzielone" i jak trzeba to robisz dodatkowe metody w swoich repositoriach

Juhas napisał(a):

zgadza się?

No mniej więcej... Tylko ja bym nie robił public IEnumerable<TEntity> GetAll(); bo to nigdy nie jest potrzebne, a łatwo tym zabić przypadkowo aplikację.
Lepiej byłoby: public IEnumerable<TEntity> GetFiltered(Expression<Func<TEntity, bool>> filter);

Czyli w klasie Client muszę mieć tylko atrybut TableName (nie wiem, czy to dobre). A nazwami pól będą po prostu nazwy właściwości. Tak chyba działają ORMy, nie?

No tak, ale dynamiczne budowanie zapytań na podstawie struktury klasy to nie powinna być odpowiedzialność repozytorium.

A czy repozytorium nie jest takim przykładem DAO?

Nie, to coś pomiędzy DAO a logiką biznesową.

scibi92 napisał(a):

W sumie to ja nie wiem czym się różni DAO od Repository

Znaczy gardzisz nawet nomenklaturą i wzorcami stworzonymi przez ekspertów Javy. Słusznie! ;)

A Repository mediates between the domain and data mapping layers, acting like an in-memory domain object collection. Client objects construct query specifications declaratively and submit them to Repository for satisfaction. Objects can be added to and removed from the Repository, as they can from a simple collection of objects, and the mapping code encapsulated by the Repository will carry out the appropriate operations behind the scenes. Conceptually, a Repository encapsulates the set of objects persisted in a data store and the operations performed over them, providing a more object-oriented view of the persistence layer. Repository also supports the objective of achieving a clean separation and one-way dependency between the domain and data mapping layers.

źródło: https://martinfowler.com/eaaCatalog/repository.html

I dodatkowo: http://commitandrun.pl/2016/05/11/Repozytorium_najbardziej_niepotrzebny_wzorzec_projektowy/

A przy okazji. Czy łączenie data mappera z domeną jest zła? Np. coś takiego:

public class Client
{
  public string FirstName { get; private set; }
  public string LastName { get; private set; }

  public Client(string firstName, string lastName)
  {

  }

  public static Client FromDB(IDataReader dataRow)
  {
    return new Client(
        dataRow["fName"].ToString(),
        dataRow["lName"].ToString());
  }
}

Czy to już łamie Single Responsibility?

Łamie SRP i uzależnia warstwę logiki biznesowej od szczegółów technologii używanej w celu dostępu do danych. To się nazywa active record i moim zdaniem jest okropne.

Czyli w grę wchodzi tylko repozytorium + DataMapper? Czy jest jeszcze jakieś inne POPRAWNE* rozwiązanie?

*poprawne oznacza niełamiące podstawowych zasad.

Pod Repozytorium nie musisz mieć DataMappera, to może być Table Data Gateway albo Data Access Object. No i repozytorium być nie musi w ogóle.

Czyli równie dobrze mogę sobie napisać klasę w stylu:

public class DataProvider
{
  public Client GetClientById(UInt64 id)
  {
    //uproszczony pseudokod
    string sql = "SELECT * FROM clients WHERE id = " + id.ToString();
    return (Client)dataReader.ExecuteQuery(sql);   
  }

  public Invoice GetInvoiceById(UInt64 id)
  {
    string sql = ... itd
  }
}

I to wystarczy? Zrobiłbym z tego singletona, dał jeszcze jeden poziom abstrakcji, żeby uniezależnić się od DBMS i wszyscy będą zadowoleni?

@Juhas: że tak się wtrącę: jak już musisz robić singletona to skorzystaj z kontenera IoC i zarejestruj w nim klasę jako singleton. Mniej pisania, lepsza organizacja, oszczędność czasu.

@grzesiek51114: czemu kontener IoC? Z tego, co rozumiem, to będzie po prostu jakaś fabryka, tak? Coś w stylu:

Singletons.Get<DataProvider>();

zamiast:

DataProvider.Instance;

Poza tym nie ma, z tego co wiem, żadnego takiego wbudowanego mechanizmu, więc musiałbym albo jakiś pobrać, albo napisać własną fabrykę. Czemu w tym wypadku pierwszy sposób miałby być lepszy?

Nie musisz bawić się w tworzenie brzydkich staticów. Poza tym dochodzą wszystkie zalety IoC związane chociażby z Dependency Injection: https://stackoverflow.com/questions/871405/why-do-i-need-an-ioc-container-as-opposed-to-straightforward-di-code Czyli generalnie niebawienie się konstruktorami, co jest strasznie irytujące kiedy musiałbyś schodkowo przekazywać parametry przez X klas. Tak masz IoC i jedyne co musisz zrobić do wyciągnąć z niego obiekt, a parametry w konstruktorze takiego obiektu uzupełnią się same. Nie uzależniasz wtedy od siebie aż tak bardzo poszczególnych klas.

Ot tworzysz zwykłą klasę, a resztą zajmuje się IoC. To jest wygodne po prostu.

Zresztą przykład masz chociażby w tym linku, który Ci podesłałem:

Wow, can't believe that Joel would favor this:

var svc = new ShippingService(new ProductLocator(), 
   new PricingService(), new InventoryService(), 
   new TrackingRepository(new ConfigProvider()), 
   new Logger(new EmailLogger(new ConfigProvider())));

over this:

var svc = IoC.Resolve<IShippingService>();

PS: Co do funkcjonalności out of box: masz przecież Unity od MS i Ninjecta czy wiele innych. Starczy dodać do projektu przez nuGet'a, a sama konfiguracja tego to naprawdę nic skomplikowanego.

@Juhas: tylko czemu chcesz robić z tego singletona?

Bo nie widzę zastosowania dla kilku instancji takiej klasy. A singleton będzie bardziej użyteczny moim zdaniem niż klasa statyczna.

Yyy... lekko w szoku jestem.
Singletony tworzy się, gdy jest potrzeba posiadania jednego obiektu danego typu, a nie profilaktycznie dla wszystkich klas. Jaki będziesz miał zysk ze zrobienia singletona, a nie normalnego tworzenia instancji? W czym konkretnie one przeszkadzają? Singleton, ze względu na długi czas życia, to proszenie się o kłopoty w przypadku operacji na bazie danych.

Juhas napisał(a):

Bo nie widzę zastosowania dla kilku instancji takiej klasy. A singleton będzie bardziej użyteczny moim zdaniem niż klasa statyczna.

Skoro nie potrzebujesz kilku instancji danej klasy to ich po prostu nie twórz. W entry poincie tworzysz sobie jedną instancję i ją po prostu przekazujesz do obiektów, które jej potrzebują. Nie ma potrzeby przyzywania tego potwora pt. "singleton pattern" :P

@somekind: nie chcę tworzyć profilaktycznie dla wszystkich klas :| Chcę stworzyć dla tej jednej mapującej dane z bazy danych.
@john_klamka: co za różnica, czy stworzę sobie singletona, czy obiekt w entry poincie, który będę przekazywał dalej? Dla mnie to drugie wyjście jest głupie. Czemu miałoby być lepsze?

Juhas napisał(a):

@somekind: nie chcę tworzyć profilaktycznie dla wszystkich klas :| Chcę stworzyć dla tej jednej mapującej dane z bazy danych.
@john_klamka: co za różnica, czy stworzę sobie singletona, czy obiekt w entry poincie, który będę przekazywał dalej? Dla mnie to drugie wyjście jest głupie. Czemu miałoby być lepsze?

jak to:

public class Singleton
{
    public string DoSomething() { //... }
}

może być "głupsze" od tego:

public sealed class Singleton
{
   private static volatile Singleton instance;
   private static object syncRoot = new Object();

   private Singleton() {}

   public static Singleton Instance
   {
      get 
      {
         if (instance == null) 
         {
            lock (syncRoot) 
            {
               if (instance == null) 
                  instance = new Singleton();
            }
         }

         return instance;
      }
   }

   public string DoSomething() { //... }
}

Porównałeś klasę nie będącą thread safe z singletonem będącym therad safe. Gdzie tu sens? Poza tym przy pobieraniu danych z bazy potrzebuję tylko jednego połączenia z bazą danych. A więc jednej instancji klasy. Zgadza się? Do takich rzeczy przecież jest stworzony singleton.

Juhas napisał(a):

Porównałeś klasę nie będącą thread safe z singletonem będącym therad safe. Gdzie tu sens? Poza tym przy pobieraniu danych z bazy potrzebuję tylko jednego połączenia z bazą danych. A więc jednej instancji klasy. Zgadza się? Do takich rzeczy przecież jest stworzony singleton.

Chodzi o implementację. Po co bawić się w pisanie zbędnego kodu, skoro singleton ma być jedyną instancją klasy, to po prostu robisz jedną instancję. Dla mnie właśnie głupie jest pisanie zbędnego kodu.

A co do połączenia z bazą - wszystko zależy od architektury projektu. Możesz przekazywać sobie tylko connection stringa, albo fabrykę połączeń. Nie bardzo widzę sytuację, w której przekazujesz sobie np. obiekt SqlConnection... no ale ja mało w życiu widziałem.

Singletona to możesz używasz jak masz klasę z jakimś configiem może, ale w innych przypadkach nie wystarczy po prostu użyć raz new?

Juhas napisał(a):

Porównałeś klasę nie będącą thread safe z singletonem będącym therad safe. Gdzie tu sens? Poza tym przy pobieraniu danych z bazy potrzebuję tylko jednego połączenia z bazą danych. A więc jednej instancji klasy. Zgadza się? Do takich rzeczy przecież jest stworzony singleton.

A jak Ty masz zamiar zarządzać tymi połączeniami i transakcjami? Jedno połączenie otwarte przez cały czas życia aplikacji? A transakcje będziesz otwierał i zamykał w metodach tego singletona?

@somekind: a jak Ty to robisz z ciekawości? Bo u mnie jest tak że korzystam z zarządzania transakcjami przez Spring i programowanie aspektowe :)

@scibi92: no ja nie mam Springa, więc po prostu rejestruję w kontenerze SessionFactory jako singletona, a Session per lifetime scope. Do tego mam zarejestrowany interceptor, który opakowuje wskazane (atrybutem) metody/klasy, i podczas ich wykonywania otwiera transakcje, wykonuje kod metody, potem zatwierdza transakcję (albo wycofuje jeśli poleciał wyjątek). Wewnątrz metody operuję po prostu na obiekcie session (Get/Load/Save/Delete/QueryOver), nie mam żadnych commitów, rollbacków, ani nawet try-catch.

Ale ja w praktyce tez tak robie. Mam TransactionManagera w Springu z odpalonym trybem adnotacji i dzięki temu nad klasami/metodami piszę

@Transactional

Spring dzięki programowaniu aspektowemu sam zarządza transakcjami tylko mu trzeba niewiele zadeklarować. Po prostu byłem ciekawy jak to wygląda w C# i tych frameworkach

somekind napisał(a):

A jak Ty masz zamiar zarządzać tymi połączeniami i transakcjami? Jedno połączenie otwarte przez cały czas życia aplikacji? A transakcje będziesz otwierał i zamykał w metodach tego singletona?

Tak. Aplikacja jest bazodanowa i ciągle korzysta z bazy danych. Łączenie się z bazą po to, żeby wykonać jedno zapytanie i potem się rozłączyć jest bez sensu. Zresztą na czymś takim przejechałem się laaata temu.

Juhas napisał(a):

@somekind: nie chcę tworzyć profilaktycznie dla wszystkich klas :| Chcę stworzyć dla tej jednej mapującej dane z bazy danych.
@john_klamka: co za różnica, czy stworzę sobie singletona, czy obiekt w entry poincie, który będę przekazywał dalej? Dla mnie to drugie wyjście jest głupie. Czemu miałoby być lepsze?

Różnica jest taka, że w przypadku drugim (przekazywanie utworzonego obiektu) masz kontrolę nad tym, gdzie jest przekazywany. W przypadku singletona, mało ogarnięty programista, może stanąć przed pokusą wywołana singletona w warstwie, w której nie powinien się ów singleton znaleźć, np. w widoku wywoła singletona, który z bazy coś tam wyciągnie i złamie zasadę separacji odpowiedzialności na poziomie komponentów (np. widok wyciągający dane z bazy).

Z mojej perspektywy sprawa prosta, albo zdajesz sobie sprawę z decyzji projektowych i ich wpływu na późniejsze zmiany, albo nie. Według mnie przed pójściem w singletony trzeba zadać sobie sporo pytań... przykładowo:
a) jaki problem ten singleton rozwiązuje?
b) skąd wziął się problem?
c) czy mam alternatywę?
d) czy z singletonem będzie skalowalne?
e) czy z singletonem będzie thread safe?
f) czy separacja classloaderów nie sprawi, że może pojawić się więcej instancji singletona :)
g) jak singleton będzie zachowywał się w klastrze
e) czy będzie dało się go "zamockować"
...

scibi92 napisał(a):

Spring dzięki programowaniu aspektowemu sam zarządza transakcjami tylko mu trzeba niewiele zadeklarować.

Jakbym chciał, to pewno znalazłbym jakiś framework dotnetowy, który dałby mi to wszystko. Tylko akurat coś takiego wolę dopasować do projektu. A "programowaniem aspektowym" zapewne też nazywasz proxy nad Twoją klasą utworzone przez kontener IoC.

Po prostu byłem ciekawy jak to wygląda w C# i tych frameworkach

Tzn. ja opisałem swój (pragmatyczny) przypadek. Ale wielu programistów otwiera sobie sesję ORMa w każdej oddzielnej metodzie, w każdej mają try-catch, i zatwierdzanie zmian... No cóż, niektórym widocznie płacą za linijkę. ;)

Juhas napisał(a):

Tak. Aplikacja jest bazodanowa i ciągle korzysta z bazy danych. Łączenie się z bazą po to, żeby wykonać jedno zapytanie i potem się rozłączyć jest bez sensu. Zresztą na czymś takim przejechałem się laaata temu.

Przejechałeś się pewnie dlatego, że implementowałeś ręcznie... tak jak chcesz zrobić teraz.

W normalnej sytuacji, połączenie jest pobierane z puli na potrzeby pojedynczej transakcji, po jej zakończeniu wraca do puli, aby inny kod mógł go użyć. A fizycznymi połączeniami zajmuje się pooler, nie programista.

To nie robicie jak ludzie, ze jest po prostu jedna, dwie metody utilsowe, które jako parametr przyjmują funkcję do wykonania na DB (z argumentem w postaci sesji)?
Tak jak tu:
https://www.jooq.org/doc/3.8/manual/sql-execution/transaction-management/

Nic nie trzeba rejestrować.
Żadnych aspektów/ procy cudów.
Testowanie proste.