Zgadzam się, ale jeżeli zauważymy, że jednak wokół prostego #create dodajemy jeszcze 2 linijki innego kodu i dzieją się bardziej zaawansowane rzeczy, to wtedy już warto wprowadzić dodatkową klasę. Na przyszłość, dla kolegów, którzy będą musieli później to naprawiać.
gogiel: często pracowałem nad projektem który już miał 2-3 lata i byłem zatrudniany żeby wprowadzić jakiś npowy ficzer na przykład. Praktycznie nie pracowałem (z 2 czy 3 wyjątkami) na green-fieldach. Najczęściej też niestety dostawałem “śmietankę” projektów których “in-house” deweloperzy woleli unikać.
Pracowałem zarówno nad prostymi projektami jak i projektami z 50+ modelami.
Zmienna lokalna dla wątku nie jest zmienną globalną! A nawet jeżeli ją tak traktujesz to w pewnych przypadkach wprowadzenie zmiennej globalnej ma na tyle plusów że znacząco przeważają nad minusami.
Dość różne. Od razu mówię, że nie mam jakiegoś niesamowicie wielkiego doświadczenia, takich które upadły miałem ze 3-4. Jeden z nich ciągnął się ok. 2 lat (ja przy nim pracowałem przez jakieś pół roku) i wolno szło głównie przez przeładowanie feature’ami - za dużo niepotrzebnych rzeczy. Myślę, że tutaj niezależnie od tego czy kod byłby wspaniały czy mierny, to skończyłoby się podobnie, bo w pewnym momencie klient się zorientował, że jednak warto posłuchać i zaczął ucinać różne rzeczy. Dlatego ten wspaniały kod i tak poszedłby do kosza. Drugi projekt jaki pamiętam był bardzo porządnie napisany (miałem okazję pracować z naprawdę top-level developerami rails, 3 osoby z core teamu i kilka innych mega dobrych programistów, byłem największym leszczem w tamtym zespole), chociaż nie używał raczej OOP w takim stylu, o jakim piszesz - testy modeli nie były prawdziwymi testami jednostkowymi, railsy były używane “standardowo”. Śmierć poprzez skończenie się kasy i niedogadanie się w kwestiach rozwoju wśród założycieli. No i ostatni jaki pamiętam był robiony przez 2 osoby i również był nieźle napisany, chociaż projekt dosyć mały na 3 miesiące kodzenia 1 jednej osoby i dodatkowy miesiąc w 2 osoby. Stoczył się przez niedziałające założenia biznesowe.
Nie znam z autopsji żadnego projektu w railsach, który musiałby być w całości przepisany ze względu na jakość kodu. Znam dużo kiepskich projektów, sam pewnie też trochę popełniłem, ale to wynikało z małego doświadczenia i braku umiejętności - gdybym wiedział wtedy jak to zrobić “poprawnie”, to zrobiłbym to dobrze również stosując KISS, YAGNI itp.
I na koniec chciałbym podkreślić jedną rzecz - nie neguję całego dorobku OOP. Jak ma to sens i nie muszę przez to napisać ton kodu, to stosuję rzeczy typu dependency injection, piszę testy jednostkowe itd., chociażby tutaj jakieś tam DI robiłem: https://github.com/travis-ci/travis-core/blob/master/lib/travis/states_cache.rb#L14. Ale czasami to nie ma dla mnie sensu i piszę coś bardzo prostego, olewam “prawdziwe testy jednostkowe”, które niczego zewnętrznego nie dotykają i idę bardzo prostą drogą.
lol
Chcesz powiedziec ze aplikacje rails nie maja problemu ze wszystkim co jest wymienione w tym poscie
Masz to napisane czarno na bialym w komentarzu Gogiela na drugiej stronie tego watku. Od siebie dorzuce ze idealnie sie pracuje z aplikacja ktora ustawia
Thread.current dla tego samego pola w kilku miejscach
I jakie to sa plusy vs przekazaniu obiektu jako parametr ?
Nie wiem czy czytamy ten sam temat ale chyba nie dociera do ciebie jakie wymagania sa postawione przez autora
Skoro nie przekonal cie Uncle Bob to nikt z nas tez tego nie zrobi dlatego mysle ze nie ma sensu dalej ciagnac tematu
Jest to tylko jedna z możliwości. Mogą też powstać w przypadku gdy rzeczywiście potrzebujemy serwisów i fabryk, tylko zostają one błędnie zaimplementowane. Nie jest to wtedy overengineering.
Protip: przeczytaj jaki nick występuje nad moim avatarem, a jaki używasz w swoich postach.
Uważam że podstawowym plusem jest właśnei to że nie musisz przekazywać jako parametr 
często przez 3-4 wywołania, jak ktoś doda callbacka to może tą zmienną tam uzyć, jak trzeba w service zapisać 5 obiektów to kazdy może jej uzyć w dowolnym miejscu kodu, zamiast przekazywać do każdego z osobna. Generalnie więc ułatwia to kodowanie znacząco.
Jedyny minus jaki Gogle wskazał z tego co zrozumiałem to to że sie nie serializuje dobrze, i tu się zgadzam, nie serializuje się dobrze. Trzeba dodać kawałek kodu który zachowuje tą zmienną przed serializacją i ustawia po odserializowaniu. Ale dokłądnie to samo trzeba by było zrobić przy przekazywaniu przez parametr.
Natomiast ty lewy ciągle nie podałeś jednego sensownego argumentu świadczącego że thread-local storage jest antywzorcem. Będę się tego czepiał do skutku, bo wreszcie się okazało że krytykować potrafisz, podczepiać się pod wzorce, ale jak przychodzi do konkretów to nie potrafisz przedstawić nawet jednego swojego argumentu na poparcie swojego zdania.
PS. @sharnik przepraszam najmocniej, automatyczne skojarzenie smile mózg nawet nie zarejestrował tego n
Herezje
Masz tupet poniewaz nie odpowiedziales na zaden moje pytanie zawarte w poprzednich postach
slowa klucz ktorych nie bede rozwijal ani tlumaczyl bo szkoda mi marnowac na to czasu to:
[quote]puma
thin
etc[/quote]
Mamy inne podejscie do programowania i nigdy nie dojdziemy do porozumienia dlatego lepiej zebysmy nie trafili na wspolny projekt 
puma, thin, etc. Nadal obsługują requesty w jednym wątku, więc w czym problem?
PS. Możemy mieć kiedyś przyjemność, światek railsowy jest mały, wtedy będziemy mogli pogadać o problemach wątków przy piwie zamiast na forum robić offtopa
zapytaj autora request_store
Na konferencjach jak najbardziej ale co do pracy mam w zwyczaju prosic o fragment kodu tworzony w ramach projektu i krotko mowiac logowanie w callbackach i Thread.current rozrzucony po apce zdecydowanie by mnie zniechecil do dalszych rozmow
ps: ja naprawde skonczylem dyskusje na ten temat
ps2: moze ktos powinien wyczyscic troche temat ?
A widzisz i wreszcie masz jakiś argument
Trzeba o tym pamiętać, ale zawsze można zrobić dokładnie to co zrobił autor gema i po prostu na początku requesta zrobić nowego hasha.
PS. Co do czyszczenia to mogę to zrobić, może jakaś sugestia od kiedy? I zgoda jeszcze z 2 uczestników dyskusji.
Najpierw to trzeba o tym wiedziec przekazywanie przez parametr nie ma takich problemow i jest zrozumiale dla poczatkujacego programisty dodatkowo latwiej sie testuje
i tak na marginesie co jak zainstalujesz gem ktory ustawia identyczna “zmienna lokalna”(lol) dla watku ?
Forum obsluguje wydzielanie do innego tematu ?
Może i troche to offtopic, ale na pewno ciekawy i w końcu troche technicznej dyskusji ;]
Callback odpala zadanie w tle (np. resque) - no i tutaj mamy przypadek, że Thread.current byłby bezużyteczny (chyba, że parametry dla niego także przekazujemy do zadania w tle).
Ja się już przejechałem na czystym Thread.current. Railsy pamiętają tam ustawienie strefy czasowej. I ta informacja zostawała pomiędzy requestami. Słabe, nie? Dlatego jeśli już jest konieczność użycia tego mechanizmu, to tylko z wymienionym już gemem request_store. Ja Thread.current (w leciwej już aplikacji) używam do prostego multi-tenancy - pamiętam wybraną platformę, a niemal cała funkcjonalność aplikacji jest oscopowana względem tej platformy. Są też gotowe gemy wspierające multi-tenancy i one też używają Thread.current - to jest akurat sensowne użycie IMHO. Czy tak jest w przypadku problemu Tuptus’a? Wydaje mi się, że jest to nieeleganckie tutaj. Zaproponowałem inny pomysł wcześniej i uważam, że odpowiednio przemyślane obiektowe wzorce projektowe mogą pomóc.
Widzę, że z prostego, wydawałoby się, zadania zrobiła się burza. Dla mnie jest to nadal proste zadanie i jeśli miałbym tworzyć serwisy do jego realizacji to już wolę pozostać przy wywoływaniu Log.create() w kontrolerze za każdym razem kiedy jest mi to potrzebne. Wykorzystując callback-i chciałem sobie ułatwić pracę. Tymczasem proponowane rozwiązanie z serwisami dodaje mi pracy i poza tworzeniem dodatkowych klas wymaga dodania w kontrolerach zamiast jednej linii kilku. To nie jest to czego szukam.
Korzystanie z Thread.current też nie za bardzo mi się podoba. Może to kwestia mojego niezbyt dużego doświadczenia ale do “grzebania” w wątkach aplikacji podchodzę z pewną taką dozą nieśmiałości.
Możecie na mnie nakrzyczeć, wykląć i co tam jeszcze ale problem rozwiązałem następująco: w ApplicationControler dodałem before_filter a w nim ustawiam zmienną klasową w klasie Log. Teraz mogę z poziomu modeli wywoływać Log.create() już bez martwienia się o informację o aktualnym użytkowniku bo Log go “zna”.
Dziękuję wszystkim za cenne uwagi. Z całą pewnością będę pamiętał o serwisach choć w tym konkretnym przypadku uznałem je za przerost formy nad treścią.
Pytanie nowicjusza odnosnie rozwiazania Tuptusa: ustawianie current usera w zmiennej klasowej Log to dobre rozwiazanie?
Wydaje mi sie, ze jezeli do zmiennej klasowej przypisze sie usera dla jakiegos requestu w ktorym uzytkownik jest zalogowany, a pozniej przyjdzie uzytkownik niezalogowany to w klasie Log ta zmienna zawierac bedzie wartosc z poprzedniego requestu. Jezeli zle mowie to bardzo prosze poprawcie.
Nie, ale akurat nie z tego powodu, o którym wspomniałeś.
Takie rozwiązanie nie jest threadsafe, więc zadziała dobrze dopóki korzystasz z serwera typu unicorn, czy passenger. Jak ktoś kiedyś tą aplikację przełączy na np. pumę, to prawdopodobnie będzie miał niemiłą niespodziankę. Ustawianie zmiennej klasowej jest globalne, tzn. może dosyć łatwo wystąpić taka sytuacja:
- user1 tworzy artykuł, zmienna klasowa w Log zostaje ustawiona na user1
- zaraz później user2 tworzy swój artykuł, request odpala się w drugim wątku, zmienna klasowa w Log zostaje ustawiona na user2
- akcja w controllerze dla user1 cały czas trwa (bo np. baza danych wolno odpowiedziała)
- Log.create() wykonuje się dla user1, ale ustawia user2, bo to jest ostatnia wartość ustawiona w zmiennej klasowej
Krótko mówiąc: jeżeli chcesz używać danych związanych z sesją w modelach, to jednak zrób to tak jak radzi Świstak i użyj Thread.current albo skorzystaj z jednej z innych opcji.
drogus dla pewnosci: na serwerze w ktorym odpowiedzi sa serwowane przez osobne procesy zmienne klasowe nie sa wspoldzielone?
http://stackoverflow.com/a/2223760 - na podstawie tego posta wydawalo mi sie, ze sa wspoldzielone.
[quote=lucas]drogus dla pewnosci: na serwerze w ktorym odpowiedzi sa serwowane przez osobne procesy zmienne klasowe nie sa wspoldzielone?
http://stackoverflow.com/a/2223760 - na podstawie tego posta wydawalo mi sie, ze sa wspoldzielone.[/quote]
Jeżeli są to osobne procesy, to raczej nie ma takiej możliwości. Zobacz np. coś takiego:
[code=ruby]class MyClass
class << self
attr_accessor :foo
end
end
MyClass.foo = ‘foo’
fork
p [:foo, Process.pid, MyClass.foo]
MyClass.foo << rand.to_s
p [:foo, Process.pid, MyClass.foo][/code]
ruby forks.rb
[:foo, 14839, "foo"]
[:foo, 14839, "foo0.8654680486216171"]
[:foo, 14840, "foo"]
[:foo, 14840, "foo0.9385197268059078"]
Ten post, do którego linka wkleiłeś nie mówi nic o oddzielnych procesach. Tam chodzi tylko o to, że taka zmienna klasowa będzie istniała przez cały czas działania procesu. To może prowadzić do różnych dziwnych zachowań, ale akurat w tym przypadku nie powinno być problemu, bo jeżeli użytkownik jest niezalogowany, to nie będzie mógł nic stworzyć (chociaż można sobie wyobrazić sytuację, gdzie ktoś dostaje do zrobienia feature, żeby niezalogowany użytkownik mógł coś dodać ).
[quote=drogus]Takie rozwiązanie nie jest threadsafe, więc zadziała dobrze dopóki korzystasz z serwera typu unicorn, czy passenger. Jak ktoś kiedyś tą aplikację przełączy na np. pumę, to prawdopodobnie będzie miał niemiłą niespodziankę. Ustawianie zmiennej klasowej jest globalne, tzn. może dosyć łatwo wystąpić taka sytuacja:
- user1 tworzy artykuł, zmienna klasowa w Log zostaje ustawiona na user1
- zaraz później user2 tworzy swój artykuł, request odpala się w drugim wątku, zmienna klasowa w Log zostaje ustawiona na user2
- akcja w controllerze dla user1 cały czas trwa (bo np. baza danych wolno odpowiedziała)
- Log.create() wykonuje się dla user1, ale ustawia user2, bo to jest ostatnia wartość ustawiona w zmiennej klasowej
Krótko mówiąc: jeżeli chcesz używać danych związanych z sesją w modelach, to jednak zrób to tak jak radzi Świstak i użyj Thread.current albo skorzystaj z jednej z innych opcji.[/quote]
Dziękuję za konkretny komentarz. Czyli kolejna poprawka kodu i jednak Thread.current. Trzeba będzie poszukać jakiejś literatury odnośnie wątków i threadsafe.
@tuptus przy thread current musisz wiedzieć że:
- Każdy request ma jeden wątek.
- Z tym że kilka requestów może mieć jeden wątek.
Więc Thread.current można używać tylko i wyłącznie do przypisywania, nie można użyć go jako licznika np. jak to pokazał lewy
Najprościej to zrobić przypisując na początku requesta do jednego klucza w Thread.current jak to robi gość w gemie