Fabryka i refleksja

zawsze możesz skompilować expression i tym samym ponieść koszt refleksji tylko raz
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/expression-trees/

A jaki jest cel takiej kombinacji? Bo wygląda jak coś, co szybko się zemści na autorze albo jego kolegach z pracy.

Nie, dlaczego? Po prostu chcę mieć pewność, że obiekt nie zostanie utworzony przez new, a tylko i wyłącznie za pomocą fabryki. Po to ma być fabryka, żeby nie tworzyć obiektów przez new. Chcę to zablokować.

Tak naprawdę to możliwe, że będzie to ostatecznie builder, a nie fabryka. Dlatego tak ważne dla mnie jest, żeby nie tworzyć tych obiektów przez new. Żeby kompilator na to nie pozwolił.

@Juhas:

chcę mieć pewność, że obiekt nie zostanie utworzony przez new

To nie wystarczy prywatny konstruktor oraz statyczna metoda Create w klasie zwracająca instancję?

No jest to jakieś rozwiązanie. Zastanawiam się tylko, czy fabryka nie byłaby lepsza pod kątem inżynieryjnym.

Pod kątem inżynieryjnym najlepsze jest posiadanie dobrego powodu do zrobienia czegoś. :)

Czemu chcesz zabronić tworzenia obiektu przez new? Boisz się, że ktoś zrobi obiekt nieprawidłowo, podając złe parametry? W takim celu wystarczy walidacja w konstruktorze. Czasami może mieć sens konstruktor prywatny + publiczna statyczna metoda fabrykująca jak wspomniał @mstl. Ale do tego też trzeba mieć powód, żeby nie zrobić tylko prostego wrappera na konstruktor.

@Juhas zapoznaj się może z takim rozwiązaniem:
http://www.codinghelmet.com/?path=howto/advances-in-applying-the-builder-design-pattern
Takie rozwiązanie co prawda nadal nie uniemożliwia używać new, ale i to da się obejść. Nawiązując trochę do przykładu z linku - publiczny powinien być tylko interfejs IPerson, a konkretna klasa powinna być zagnieżdżona w klasie Builder:

public interface IPerson
{
	string Name { get; }
}

public class Builder
{
	public IPerson Build()
	{
		return new Person()
		{
			Name = "Juhas"
		};
	}

	private class Person : IPerson
	{
		public string Name { get; set; }
	}
}

Tylko zastanów się, czy na pewno czegoś takiego potrzebujesz ;)

Wydaje mi się ze dobrym pomysłem jest konstruktor internal; i zamknąć całość w dll'ce.
Ew klasyczny factory patern.
Autor wspomniał że obiekty są używane w całym projekcie...ale chyba using'i nie stanowią problemu, chociaż ... cierpi tak zw "decoupling".

Albo już wspomniany prywatny konstruktor i statyczna "metoda fabrykująca".

P

somekind napisał(a):

Pod kątem inżynieryjnym najlepsze jest posiadanie dobrego powodu do zrobienia czegoś. :)

Czemu chcesz zabronić tworzenia obiektu przez new? Boisz się, że ktoś zrobi obiekt nieprawidłowo, podając złe parametry? W takim celu wystarczy walidacja w konstruktorze. Czasami może mieć sens konstruktor prywatny + publiczna statyczna metoda fabrykująca jak wspomniał @mstl. Ale do tego też trzeba mieć powód, żeby nie zrobić tylko prostego wrappera na konstruktor.

Więc tak. Na początku chciałem mieć fabrykę generyczną. W stylu:

Item i = Factory.Instance.CreateItem<T>(jakiesParametry);

A skoro za tworzenie obiektu ma odpowiadać fabryka, to chcę uniemożliwić tworzenie przez new, bo w przyszłości coś się może rozjechać. Niech teraz ktoś coś utworzy przez new w kilku miejscach w kodzie. A za dwa miesiące zmieni się ciało metody CreateItem w fabryce w jakimś stopniu. Tu zachodzi obawa, że wtedy tamte obiekty tworzone przez new będą w jakiś sposób źle skonstruowane. Dlatego też chcę zabronić tworzenia tych obiektów przez new. W ostateczności mógłbym zrobić to tak, że konstruktor przyjmowałby obiekt fabryki i sprawdzał, czy fabryki są sobie równe, coś w stylu:

public ConcreteItem(Factory f, inneParametry, [CallerMemberName] string memberName = "")
{
    if((f != Factory.Instance) || (memberName != "CreateItem))
        throw new Exception("Nie o takie Polskie walczyłem!");
}

no, ale sam widzisz, co to za kod ;)
Nie dość, że naprawdę kiepski to jeszcze zmniejsza możliwość nieprawidłowego utworzenia, ale nie eliminuje jej.

Pomysł z budowniczym, gdzie klasa jest wewnętrzną klasą budowniczego jest niezły, ale tych klas będzie kilka i chciałbym, żeby były normalnymi klasami, a nie wewnętrznymi.

I teraz załóżmy taką sytuację. Mam 3 klasy konkretne: A, B i C.

Tworzenie każdej z nich może (choć pewnie nie będzie) być lekko inne, np:

A a = new A(p); //jakiś parametr
if(a.cosTam)
  jakisInnyObiekt.Add(a);

B b = new B();

InnaKlasa k = new InnaKlasa();
C c = new C(k);
jakisInnyObiekt.Add(c);

Wiem, że w tym przypadku mógłbym porobić po prostu odpowiednie konstruktory, ale:

• utrudni to tworzenie tych obiektów i przy okazji zaciemni nieco kod
• nie chcę żeby te klasy wiedziały cokolwiek o "jakisInnyObiekt" - mają być zupełnie rozdzielone
• to tylko pierwszy, lepszy przykład o jakim pomyślałem, w rzeczywistości może to wyglądać zupełnie inaczej

I tu najlepszym rozwiązaniem wydaje się być takie połączenie fabryki z budowniczym.

Żeby klient wywołał tylko:

Item a = Factory.Instance.CreateItem<A>();
Item b = Factory.Instance.CreateItem<B>();
Item c = Factory.Instance.CreateItem<C>();

I tu nie ma problemu, że jakiś element zostanie źle utworzony i rozwali program. I klienta nie interesuje jak jest obiekt tworzony.

Zaznaczam, że póki co nie mam żadnego konkretnego problemu, po prostu patrzę dość przyszłościowo (że problem wymagający innego tworzenia obiektu może się pojawić z czasem). Chcę od samego początku dobrze przemyśleć ten mechanizm i zaimplementować. Żeby w razie czego nie trzeba było przekopywać połowy aplikacji.

W C++ użyłbym po prostu friendsów do tego. I nie byłoby sprawy. Ale w C# nie ma takiego mechanizmu, więc muszę wykombinować coś innego ;)

Najprostszym i chyba najbardziej naturalnym rozwiązaniem wydaje się tu refleksja. I tego bym użył, gdyby refleksja nie była tematem sporów. Więc szukam innych sposobów na osiągnięcie celu, jednak mam też pewne własne ograniczenia - jak np. to, że chcę żeby te klasy nie były zadeklarowane wewnątrz innych.

Wydaje mi się, że chcesz mieć ciastko i zjeść ciastko. Klasa zaprzyjaźniona to dla mnie gwałt na OOP, już bez wdawania się w szczegóły.
Najlepsza odpowiedź już tutaj padła od @somekind

Pod kątem inżynieryjnym najlepsze jest posiadanie dobrego powodu do zrobienia czegoś.

A póki co piszesz:

Zaznaczam, że póki co nie mam żadnego konkretnego problemu, po prostu patrzę dość przyszłościowo

Zastosuj proste rozwiązanie (ale nie prostsze! ;) i czas pokaże, czy było dobre. A jak nie chcesz tworzenia instancji przez new, to zastosuj sobie statyczna analizę, która będzie grzecznie informować, żeby takiej konstrukcji nie używać ;)

mstl napisał(a):

Wydaje mi się, że chcesz mieć ciastko i zjeść ciastko. Klasa zaprzyjaźniona to dla mnie gwałt na OOP, już bez wdawania się w szczegóły.
Najlepsza odpowiedź już tutaj padła od @somekind

Pod kątem inżynieryjnym najlepsze jest posiadanie dobrego powodu do zrobienia czegoś.

A póki co piszesz:

Zaznaczam, że póki co nie mam żadnego konkretnego problemu, po prostu patrzę dość przyszłościowo

Wyrwałeś połowę mojej wypowiedzi. Spójrz na całość:

Zaznaczam, że póki co nie mam żadnego konkretnego problemu, po prostu patrzę dość przyszłościowo (że problem wymagający innego tworzenia obiektu może się pojawić z czasem). Chcę od samego początku dobrze przemyśleć ten mechanizm i zaimplementować. Żeby w razie czego nie trzeba było przekopywać połowy aplikacji.

To jest mój problem.

Zastosuj proste rozwiązanie (ale nie prostsze! ;) i czas pokaże, czy było dobre. A jak nie chcesz tworzenia instancji przez new, to zastosuj sobie statyczna analizę, która będzie grzecznie informować, żeby takiej konstrukcji nie używać ;)

Możesz to rozwinąć?

@Juhas: odnośnie analizy kodu - jeżeli masz R# możesz np. skorzystać z Options -> Code Inspection -> Custom Patterns i możesz dodać sobie, że na new A dostaniesz Warning lub Error. Takie ustawienie później możesz wrzucić do repo. My w jednym z projektów wyświetlamy komunikat, gdy ktoś użyje ViewBag (z ASP.MVC) :D

Juhas napisał(a):

A skoro za tworzenie obiektu ma odpowiadać fabryka, to chcę uniemożliwić tworzenie przez new, bo w przyszłości coś się może rozjechać. Niech teraz ktoś coś utworzy przez new w kilku miejscach w kodzie. A za dwa miesiące zmieni się ciało metody CreateItem w fabryce w jakimś stopniu. Tu zachodzi obawa, że wtedy tamte obiekty tworzone przez new będą w jakiś sposób źle skonstruowane. Dlatego też chcę zabronić tworzenia tych obiektów przez new.

Jeśli fabryka ma tworzyć te obiekty, to i tak musi użyć new, więc jeśli nie zadbasz o spójność obiektu w konstruktorze, to równie dobrze coś może się rozjechać w tych obiektach z fabryki.
A źle skonstruowane obiekty, skoro będą złe, to nie będą działać...

Rozumiem, że obiekt jest skomplikowany do stworzenia, więc chcesz dla bezpieczeństwa napisać do niego fabrykę czy tam buldera. Informujesz o tym kolegów z zespołu, i od tej pory używacie tamtych klas, a nie bezpośrednio new. Jak dla mnie po sprawie, jeśli ktoś ręcznie zbuduje zły obiekt, to nie będzie mu działał i nie skończy swojego taska.

Z drugiej strony, jeśli obiekt jest trudny do utworzenia, to może po prostu jest za duży i należałoby go podzielić?

Tworzenie każdej z nich może (choć pewnie nie będzie) być lekko inne, np:

Kopiuj

A a = new A(p); //jakiś parametr
if(a.cosTam)
  jakisInnyObiekt.Add(a);

Tworzenie obiektu A, to tworzenie A, a nie umieszczanie go w jakimś innym obiekcie. Jeśli jest taka potrzeba, to powinieneś pisać fabrykę dla jakisInnyObiekt, a nie dla A.

Zaznaczam, że póki co nie mam żadnego konkretnego problemu, po prostu patrzę dość przyszłościowo (że problem wymagający innego tworzenia obiektu może się pojawić z czasem). Chcę od samego początku dobrze przemyśleć ten mechanizm i zaimplementować. Żeby w razie czego nie trzeba było przekopywać połowy aplikacji.

No to zrób fabrykę/buildera i jej używaj. Jeśli ktoś jej nie użyje, a potem spowoduje to problemy w aplikacji, to trudno, i tak łatwiej będzie to naprawić niż wymyślić teraz jakieś rozwiązanie, które poza gmatwaniem kodu będzie się i tak dało obejść.
Oczywiście możesz bawić się w coś takiego:

class Factory
{
    public Item CreateItem()
    {
        return new Item();
    }
}

class Item
{
    public Item()
    {
        var stackTrace = new StackTrace();
        var methodBase = stackTrace.GetFrame(1).GetMethod();

        if (methodBase.DeclaringType != typeof(Factory))
        {
            throw new ApplicationException("Wracaj do domu łajdaku!");
        }
    }
}

Tylko jakbym ja zobaczył coś takiego w kodzie w pracy, to bym od razu skasował. :P

Najprostszym i chyba najbardziej naturalnym rozwiązaniem wydaje się tu refleksja. I tego bym użył, gdyby refleksja nie była tematem sporów.

Ale co refleksja tu rozwiąże? Przecież każdy może jej użyć i wywołać prywatny konstruktor, dokładnie tak jak Ty w swojej fabryce.

Uważam, że to zły pomysł.
To próba zmuszenia języka do bycia czymś, czym nie jest.
Gdyby C# miał być tak używany, to by było wspierane na poziomie języka (ten a ten obiekt ma monopol na tworzenie tych obiektów) i nie wymagało symulowania wymyślnymi sztuczkami.
Te sztuczki to jest tzw. evil code, który może być napisany nawet zręcznie, ale jest nieoczywisty.

1. Miałem jeszcze jakiś pomysł z zagnieżdżaniem fabryki w klasach, ale to od razu eliminuje.

Nie wiem dlaczego, bo spośród rozpatrywanych sposobów ten jest chyba jeszcze najbardziej "przyzwoity", to znaczy szanujący założenia języka i nieprzekombinowany.

Pytanie jest takie: gdy patrzę na ConcreteClassA i nie mogę znaleźć konstruktora, skąd właściwie mam wiedzieć, że mam obowiązek użyć fabryki i gdzie ona jest?
Muszę np. zerknąć do dokumentacji, ewentualnie wydedukować sobie na podstawie lektury zastanego kodu.

A może po prostu popatrzeć na to odwrotnie.
Skoro do odpalenia obiektu nieodzowna jest fabryka, może niech jego konstruktor bierze ową fabrykę jako swój argument.

public ConcreteClassA(MyClassFactory factory) 
{
	// i tu ustawianie wartości, inicjalizowanie zasobów itd.
}

Oczywiście w takim wypadku to już nie jest wzorzec fabryka i należałoby zmienić tę mylącą nazwę.

Np. na MyClassHydrator (który "nastrzykuje" tworzony obiekt wartościami, strategiami, cokolwiek mu trzeba), albo MyClassInitializer, albo MyClassBootstrapper.

public ConcreteClassA(MyClassInitializer initializer) 
{
	initializer.initialize(this)
	// hydrator.hydrate(this)
	// bootstrapper.bootstrap(this)
}

Dla każdego kolegi jest wtedy z automatu oczywiste, że to jedyny sposób tworzenia obiektów tej klasy.

I nie trzeba szukać, gdzie ta fabryka w zasadzie jest położona, nie trzeba robić czarcich sztuczek z refleksją, sztucznie wydzieloną dll-ką, rzucać wyjątków w trakcie jazdy, konfigurować ReSharpera i w ogóle "bohatersko walczyć z problemami nieznanymi w normalnych ustrojach"...

somekind napisał(a):

Tworzenie obiektu A, to tworzenie A, a nie umieszczanie go w jakimś innym obiekcie. Jeśli jest taka potrzeba, to powinieneś pisać fabrykę dla jakisInnyObiekt, a nie dla A.

No tak to ma być robione. Fabryka/builder ma być generyczna. Ma posiadać jedną publiczną metodę:

CreateItem<T>

I na podstawie przekazanego typu ma umieć odpowiednio stworzyć obiekt. Obiekty pewnych typów muszą być dodawane do listy, a innych typów nie. Tak jak pisałem - normalnie przekazywałbym tą listę w konstruktorze klasy, ale te klasy mają niczego nie wiedzieć o tym. Poza tym problem jest nieco bardziej skomplikowany niż go przedstawiam. Dlatego też w grę wchodzi tylko builder/fabryka.

Ale co refleksja tu rozwiąże? Przecież każdy może jej użyć i wywołać prywatny konstruktor, dokładnie tak jak Ty w swojej fabryce.

Prywatny konstruktor. Dzięki czemu nie można utworzyć obiektu. Jeśli nie można utworzyć obiektu, to jest już wtedy bardzo duża szansa na to, że gościu przeczyta jednak dokumentację w kodzie przy tym konstruktorze i zorientuje się, że fabryka/builder ma to tworzyć :)
Co sądzicie o takim rozwiązaniu. Ma to sens, czy nie? W tym przypadku narzutem refleksji się w ogóle nie przejmuję.

Ale może faktycznie walczę z wiatrakami i chcę zrobić coś nadgorliwego. Może faktycznie powinienem zostawić to new w spokoju i wpisać w dokumentacji, żeby tak nie tworzyć. Ale wydaje mi się to nie do końca poprawnym rozwiązaniem, bo powoduje wieloznaczność i niespójność aplikacji.

Przeszły mi przez myśl wcześniej rozwiązania ze stack trace i wciskaniem fabryki do konstruktora. Ale jeśli chodzi o stack trace... no to wiadomo ;) A fabryka do konstruktora - cały czas chciałem mieć faktyczną fabrykę/buildera i tu już pojawiał się problem.

Więc czekam jeszcze co myślicie o użyciu refleksji w tym przypadku. I albo zrobię tak, albo po prostu niech te klasy będą wnętrznościami fabryki, albo oleję to new.

Juhas napisał(a):

Więc czekam jeszcze co myślicie o użyciu refleksji w tym przypadku.

Ja bym nie używał.

Z trzeciej strony ;) skoro dostaliśmy refleksję, fajnie by było czasem jej użyć.

Tak samo tłumaczył chłopiec, któremu na izbie przyjęć trzeba było wyjąć pięciozłotówkę z miejsca, z którego - jak się okazało - trudno ją wyciągnąć samodzielnie.

- Fajnie czasem użyć - zgodził się lekarz dyżurny - ale jednak najpierw trzeba się dwa razy zastanowić.

Ogólna zasada jest taka, że po refleksję sięgamy, kiedy obiekt czy kod, wobec którego stosujemy refleksję, powstaje (jest budowany) później, niż kod stosujący refleksję.

Czyli jeżeli byś np. pisał nowy, najszybszy na świecie serializator do jsonów albo własną bibliotekę ORM - można sięgnąć po refleksję, bo nie możemy wiedzieć z góry, jakie obiekty ktoś nam poda.

Stosowanie jej w obrębie kodu, który jest w całości posiadany i kontrolowany przez nas, to już mocno podejrzane.

Problemy z refleksją są nie tylko wydolnościowe; te można łagodzić. Z samej natury wnosi ona dodatkową warstwę komplikacji - nie tylko dorzuca syfu do śladów stosu, ale też utrudnia działanie różnych narzędzi (np. do statycznej analizy kodu albo obfuskatorów...)

Juhas napisał(a):

No tak to ma być robione. Fabryka/builder ma być generyczna. Ma posiadać jedną publiczną metodę:

CreateItem<T>

I na podstawie przekazanego typu ma umieć odpowiednio stworzyć obiekt. Obiekty pewnych typów muszą być dodawane do listy, a innych typów nie.

Generyczność to inaczej uogólnienie. Jeżeli jedne obiekty mają być tworzone inaczej niż drugie, to to jest zaprzeczenie generyczności.

Prywatny konstruktor. Dzięki czemu nie można utworzyć obiektu. Jeśli nie można utworzyć obiektu, to jest już wtedy bardzo duża szansa na to, że gościu przeczyta jednak dokumentację w kodzie przy tym konstruktorze i zorientuje się, że fabryka/builder ma to tworzyć :)

Albo przeczyta i stwierdzi, że ta fabryka mu wcale nie pomaga, i wywoła konstruktor.

Ale może faktycznie walczę z wiatrakami i chcę zrobić coś nadgorliwego. Może faktycznie powinienem zostawić to new w spokoju i wpisać w dokumentacji, żeby tak nie tworzyć. Ale wydaje mi się to nie do końca poprawnym rozwiązaniem, bo powoduje wieloznaczność i niespójność aplikacji.

Nie powoduje, dopóki tego pilnujesz. Jeśli ktoś napisze niepoprawny kod, odrzuć mu go na review.

somekind napisał(a):

Juhas napisał(a):

No tak to ma być robione. Fabryka/builder ma być generyczna. Ma posiadać jedną publiczną metodę:

CreateItem<T>

I na podstawie przekazanego typu ma umieć odpowiednio stworzyć obiekt. Obiekty pewnych typów muszą być dodawane do listy, a innych typów nie.

Generyczność to inaczej uogólnienie. Jeżeli jedne obiekty mają być tworzone inaczej niż drugie, to to jest zaprzeczenie generyczności.

Zgoda, ale posługuję się narzędziami, które mam.

Prywatny konstruktor. Dzięki czemu nie można utworzyć obiektu. Jeśli nie można utworzyć obiektu, to jest już wtedy bardzo duża szansa na to, że gościu przeczyta jednak dokumentację w kodzie przy tym konstruktorze i zorientuje się, że fabryka/builder ma to tworzyć :)

Albo przeczyta i stwierdzi, że ta fabryka mu wcale nie pomaga, i wywoła konstruktor.

No nie wywoła, bo jest prywatny. Więc albo musiałby zmienić konstruktor na publiczny, albo użyć refleksji. W obu przypadkach podejrzewam, że nawet początkujący programista by uznał, że to są złe drogi ;)

Ale może faktycznie walczę z wiatrakami i chcę zrobić coś nadgorliwego. Może faktycznie powinienem zostawić to new w spokoju i wpisać w dokumentacji, żeby tak nie tworzyć. Ale wydaje mi się to nie do końca poprawnym rozwiązaniem, bo powoduje wieloznaczność i niespójność aplikacji.

Nie powoduje, dopóki tego pilnujesz. Jeśli ktoś napisze niepoprawny kod, odrzuć mu go na review.

Review... Piękny byłby to świat... ;)

Wiem :D, jeśli twoja fabryka jest singletonem, to możesz ją inicjalizować w statycznych konstruktorach "klas bez new". "klasy bez new" miały by prywatny konstruktor a do fabryki przekazywały by func albo specjalny interfejs.

class NoNew : NoNewBase
{
    private static object initObject = new NoNew ();
    static NoNew()
    {
           NoNewBase.Factory.Add( (args) => new MoNew(args) );
           initObject = null; //clean up
    }

    //initialize
    private noNew(object args)
    {
         // force static constructor and do nothing
    }
    private noNew(object args)
    {
         // do something
    }
}

Tylko pamiętaj żeby fabrykę zrobić przez if(instance==null)instance=new inst(); return instance i nie polegać na kolejności dodawania elementów bo ona jest nie określona dla konstruktorów statycznych.