Witam, ostatnio jestem w trakcie lektury książki R.C Martin'a Czysty kod, i jest tam pełno kodu, który rozbijany jest na jedno dwu liniowe przykłady o dość specyficznych opisowych nazwach. Postanowiłem napisać kawałek kodu wg Jego schematu, akurat miałem refaktoryzować model słownika do pewnej aplikacji finansowej, gdzie kod jest fatalny, ale małymi krokami staram się to zmieniać i tak oto napisałem fragmęt:
<?php class CriteriumDictionary { const TYPE_ACTIVITY = 0, TYPE_SIZE = 1, TYPE_TRADE = 2, TYPE_VIOVODE = 3, TYPE_CAPITAL = 4, TYPE_REGION = 5, TYPE_INCOME = 6, NO_SUBCRITERIUM = 0 ; public http://www.php.net/static $type = [ self::TYPE_ACTIVITY => 'Działalność', self::TYPE_SIZE => 'Rozmiar', self::TYPE_TRADE => 'Branża', self::TYPE_VIOVODE => 'Województwo', self::TYPE_CAPITAL => 'Kapitał', self::TYPE_REGION => 'Region', self::TYPE_INCOME => 'Przychód' ]; public http://www.php.net/static $activity = [ 1 => 'Usługowa', 2 => 'Produkcyjna', 3 => 'Handlowa' ]; public http://www.php.net/static $size = [ 1 => [ 'name' => 'Mała', 'desc' => 'poniżej 50 pracowników' ], 2 => [ 'name' => 'Średnia', 'desc' => 'od 50 do 249 pracowników' ], 3 => [ 'name' => 'Duża', 'desc' => 'od 250 do 500 pracowników' ], 4 => [ 'name' => 'Bardzo duża', 'desc' => 'powyżej 500 pracowników' ] ]; public http://www.php.net/static $trade = [ 1 => 'Bankowość', 2 => 'Budownictwo', 3 => 'Energetyka i ciepłownictwo', 4 => 'Handel', 5 => 'Logistyka, transport', 6 => 'Przemysł', 7 => 'IT i telekomunikacja', 8 => 'Ubezpieczenia', 9 => 'Usługi dla ludności', 10 => 'Usługi dla biznesu', 11 => 'Outsourcing', 12 => 'Automotive', 13 => 'Chemiczna', 14 => 'Elektroniczna i elektrotechniczna', 15 => 'Materiały budowlane', 16 => 'Medyczna i farmaceutyczna', 17 => 'Metalowa i metalurgiczna', 18 => 'Spożywcza' ]; public http://www.php.net/static $voivode = [ 1 => 'zachodniopomorskie', 2 => 'pomorskie', 3 => 'warmińsko-mazurskie', 4 => 'podlaskie', 5 => 'lubuskie', 6 => 'wielkopolskie', 7 => 'kujawsko-pomorskie', 8 => 'mazowieckie', 9 => 'łódzkie', 10 => 'lubelskie', 11 => 'dolnośląskie', 12 => 'opolskie', 13 => 'śląskie', 14 => 'małopolskie', 15 => 'podkarpackie', 16 => 'świętokrzyskie' ]; public http://www.php.net/static $capital = [ 1 => 'Przewaga kapitału polskiego (powyżej 50%)', 2 => 'Przewaga kapitału zagranicznego (powyżej 50%)' ]; public http://www.php.net/static $region = [ 1 => [ 'name' => 'Wschód', 'voivodes' => [10, 15, 4, 16], ], 2 => [ 'name' => 'Zachód', 'voivodes' => [5, 6, 1, 11, 12] ], 3 => [ 'name' => 'Północ', 'voivodes' => [7, 2, 3] ], 4 => [ 'name' => 'Południe', 'voivodes' => [13, 14] ], 4 => [ 'name' => 'Centrum', 'voivodes' => [8, 9] ], ]; public http://www.php.net/static $income = [ 1 => 'do 100 mln PLN', 2 => 'od 100 do 1000 mln PLN', 3 => 'powyżej 1000 mln PLN' ]; public http://www.php.net/static function get($criterium, $subcriterium=0) { self::throwExceptionIfCriteriumIsNotExists((int)$criterium); return self::getOneSubcriteriumIfSelectedOrAll((int)$criterium, (int)$subcriterium); } public http://www.php.net/static function getNames($criterium) { self::throwExceptionIfCriteriumIsNotExists((int)$criterium); return self::extractNamesForCriterium((int)$criterium); } private http://www.php.net/static function extractNamesForCriterium($criterium) { $subcriterias = self::getOneSubcriteriumIfSelectedOrAll((int)$criterium, self::NO_SUBCRITERIUM); switch ($criterium) { case self::TYPE_SIZE: case self::TYPE_REGION: $names = []; foreach ($subcriterias as $subcriteriumId => $subcriterium) { $names[$subcriteriumId] = $subcriterium['name']; } default: $names =& $subcriterias; break; } return $names; } private http://www.php.net/static function throwExceptionIfCriteriumIsNotExists($criterium) { if (self::isCriteriumExists($criterium) === false) { throw new Exception('Kryterium(numer '.$criterium.'), które jest wymaganie nie istnieje w słowniku'); } } private http://www.php.net/static function getOneSubcriteriumIfSelectedOrAll($criterium, $subcriterium) { if (self::isSelectedSubcriterium($subcriterium)) { self::throwExceptionIfSubcriteriumIsNotExists($criterium, $subcriterium); return self::getData($criterium, $subcriterium); } return $criterium; } private http://www.php.net/static function isSelectedSubcriterium($subcriterium) { return $subcriterium > 0; } private function throwExceptionIfSubcriteriumIsNotExists($criterium, $subcriterium) { if (isSubcriteriumExists($criterium, $subcriterium) === false) { throw new Exception('Subkryterium '.$subcriterium.' dla kryterium '.$criterium.' które jest wymaganie nie istnieje'); } } private http://www.php.net/static function getData($criterium, $subcriterium=0) { if (!http://www.php.net/isset(self::$type[$criterium])) { return []; } if (http://www.php.net/isset(self::$type[$criterium][$subcriterium])) { return self::$type[$criterium][$subcriterium]; } return self::$type[$criterium]; } public http://www.php.net/static function isCriteriumExists($criterium) { return http://www.php.net/isset(self::$type[$criterium]); } public http://www.php.net/static function isSubcriteriumExists($criterium, $subcriterium) { return http://www.php.net/isset(self::$type[$criterium]) && http://www.php.net/isset(self::$type[$criterium][$subcriterium]); } }
if (http://www.php.net/isset($this->accountData[12]) && !http://www.php.net/empty($this->accountData[12])) { //do something }
if ($this->companyNameIsFilled()) { //do something }
nie:
if (isset($this->accountData[12]) && !empty($this->accountData[12])) {
a poprostu
if (!empty($this->accountData[12])) {
Zas co do reszty twojego kodu to strasznie kola w oczy bledy w angielskim oraz nazwy funkcji,eg:
nie:
Is criterium exists
a:
Does criterium exist
Analogicznie cala reszta
Rowniez dlugosc nazw jest zdziebko zadluga
Zamiast
throwExceptionIfCriteriumIsNotExists
daj poprostu
checkIfCriteriumExists
Ponadto generujesz wszystko na static a potem jedno nie jako static, ale i tak wywolujesz jak static
Dodatkowo raz uzywasz stalych a raz nie
Za duzo tez rzeczy trzymasz jako wartosci w klasie. W zasadzie wiekszosc (jak nie wszystkie) powinny lezec w tabelach slownikowych
No tak, nie zastanowiłem się,
Co do tabel słownikowych tak była napisana aplikacja a jako, że jestem w trakcie refaktoryzacji kodu, chętnie przyjmę każdą poradę.
Mam kolegę który jest przeciwnego zdania, żeby nie trzymać takich rzeczy które raczej bardzo rzadko się zmieniają w tabelach w BD, że jest to strata optymalizacji.
Ten jeden brak statica to tez błąd, chciałem wysłać posta a musiałem wychodzić i to wszystko stąd, przerpaszam za takie olewnictwo
A jak byś rozwiązał taki słownik, zrobił tabelę do każdego typu osobno i zrobił klasę abstrakcyjną lub interfejs, czy razem w jednej tabeli trzymał wszystko i jedną klasa to obsługiwał.
Niektóre wartości w słowniku mają tylko nazwe niektore wiecej informacji.
Jakby chciec zrobic to super poprawnie, to na kazda tabele slownikowa by sie przydala klasa - model.
Zas ja osobiscie na taka rzecz uzywam jednej klasy, ktora zwraca mi co trzeba na podstawie parametru bedacego nazwa slownika.
Twoje tabele slownikowe powinny zawierac pola ID, NAME, DESC i zaspokoi to potrzeby w 99%
Raz widze masz jeszcze tam wojewodztwa. To albo dodac kolejne pole do tej tabeli i po przecinku ID wojewodztw albo tabele laczaca.
Zas co do optymalnosci na ktora skarzy sie kolega - no po to takie rzeczy trzyma sie w cache a nie pobiera za kazdym razem
@nospor
Zgodzę się z tabelami słownikowymi, pod warunkiem nie trzymania tam AI, poza tym, imo i tak lepiej trzymać stałą w obiekcie, zdecydowanie lepiej jest zrobić warunek:
class SomeDictionary { const MIGHTY_PROPERTY = 1; } if($someObject->someMethod(SomeDictionary::MIGHTY_PROPERTY)){/**/}
if($someObject->someMethod(1)){/**/}
A co więcej PHP ma http://php.net/manual/en/class.splenum.php
Co z tego skoro ma wmaganie (PECL spl_types >= 0.1.0)
Ano nic Używam od dawna tego https://github.com/garoevans/php-enum
Jak jest natywnie to korzysta, jak nie to tworzy wrapper.
To ta klaska sluzy tylko i wylacznie do sprawdzania czy istnieja stale. Rety... czego to ludzie nie wymysla....
Jak przechodzisz z Javy to właśnie enumeratorów brakuje. Lepsze to niż interface symylujący.
No tak... java... jedno slowo warte 1000 innych
Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)