Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: [Klasa][PHP7] Cache
Forum PHP.pl > Inne > Oceny
KsaR
Źródło:
https://github.com/KsaR99/php-cache

Plik: class.Cache.php, zawiera klasę.
Plik: cache.php, to przykładowe zastosowanie.

Klas cache są miliony jednak chciałem stworzyć swoją własną, prostą. biggrin.gif

Do oceny:
  • Użyteczność
  • Co można poprawić żeby sobie/innym życia nie skomplikować
  • Jakość kodu (ciągle uczę się nią polepszać)


Zastanawiam się nad metodą do kasacji cache,
Jednak cache zazwyczaj się nie kasuje a podmienia.
Warto dodać? Jeśli tak to prosił bym też o przykład - po co.

Wszelkie uwagi mile widziane. tongue.gif
kapslokk
Przydała by się możliwość cachowania innych typów danych. ;]
Pyton_000
Kompresja gz. Do wywalenia. Jedynie spowalnia całość.

mrc
Więcej dokumentacji, niż właściwego kodu biggrin.gif
KsaR
Cytat(kapslokk @ 8.06.2016, 19:19:32 ) *
Przydała by się możliwość cachowania innych typów danych. ;]

Postaram się dodać. tongue.gif

Cytat(Pyton_000 @ 8.06.2016, 19:33:23 ) *
Kompresja gz. Do wywalenia. Jedynie spowalnia całość.

Domyślnie jest wyłączona więc nie powinno spowalniać. (Chyba że w ms).

Cytat(mrc @ 8.06.2016, 19:53:58 ) *
Więcej dokumentacji, niż właściwego kodu biggrin.gif

Przecież jest napisane "... prostą", ważne że robi swoje. tongue.gif
mrc
Co do phpdoc dla metod: całkiem niepotrzebny jest dla parametrów, dla których podajesz typ w kodzie. Zaciemnia to kod, dorzuca dodatkowy szum informacyjny, może prowadzić do problemów przy refactoringu - przy zmianie zachowania funkcji możesz zapomnieć o zmianie komentarza, który później będzie wprowadzał w błąd.


-- Edit --

"if you want use compression mode"

vs

"if you want to use compression mode".

-- Edit --

Do tego throw Exception() przy użyciu namespace może wyrzucać błąd nieznalezienia klasy. Rzucaj \Exception albo bardziej oczywiste wyjątki jak \InvalidArgumentException.
com
Dobre IDE podpowie, że typ się zmienił, a Storm używa phpDoca do podpowiadania, tylko mnie rażą te nazwy typów UpperCasem.

No ale skoro może spowalniać to poco wgl ja trzymać jako opcje?

Cytat
Do tego throw Exception() przy użyciu namespace może wyrzucać błąd nieznalezienia klasy.


Nawet nie może tylko wyrzuci smile.gif

komentarze to nie
  1. /**
  2. *
  3. *
  4. */

tylko
  1. /**
  2.  *
  3.  *
  4.  */


No i czemu taka archaiczna konwencja nazewnictwa?
Gdzie PSR

Nazywaj jakoś sensownie te commity wink.gif

Z ścieżki i innych zrobiłbym VO, bo czemu klasa cache ma za to odpowiadać, czy one są poprawne, jak to nie jej rola.

I poco ustawiać wartości domyślne, parametrom którym wymuszasz ich zmianę?

Zaraz, ja muszę stworzyć sobie plik żeby do niego pisać?

poza tym else to zło, naprawdę nie można było

  1. if (!isset($path) || !file_exists($path)) {
  2. throw new \Exception('bla bla');
  3. }
  4.  
  5. $this->path = $path;


Btw treść commita to nie komentarz, bo wpisujesz tam to co ma znaczenie i utrudniasz sobie i innym czytanie commitów wink.gif

No i nazywaj te parametry tak żeby mówiły co robią, a nie np $compress - co sugeruje że coś jest skompresowane a nie wskazuje na flage itp

Skoro używasz dobrodziejstw php 7 to poco sprawdzać isset($compress) && $compress questionmark.gif bool to zawsze bool

zrób sobie jakaś metodę do budowania właściwej ścieżki, bo tak ciężko to testować/refaktorwać potem

Zrobiłem Ci pull requesta, bo za dużo tego było wink.gif
Pyton_000
Skoro mamy już $extension i $isCompressed to warto by było nadać im jakiś default. Compressed na false, a ext. nie wiem może ".dat"
com
Ok uwzględnię, zastanawiałem się nad tym smile.gif
KsaR
Cytat(com @ 9.06.2016, 00:22:44 ) *
(...), tylko mnie rażą te nazwy typów UpperCasem.

No ale skoro może spowalniać to poco wgl ja trzymać jako opcje?

(...)No i czemu taka archaiczna konwencja nazewnictwa?
Gdzie PSR

(...)Zaraz, ja muszę stworzyć sobie plik żeby do niego pisać?

(...)No i nazywaj te parametry tak żeby mówiły co robią, a nie np $compress - co sugeruje że coś jest skompresowane a nie wskazuje na flage itp

(...)Zrobiłem Ci pull requesta, bo za dużo tego było wink.gif

Dzięki, wiele zmian dodałem w ostatnich 2 commitach.
Nie znalazłem nigdzie w PSR czy używać małymi czy dużymi te nazwy typów (było tylko o parametrach).
Jeśli ktoś poda konkretny link do PSR gdzie pisze że trzeba małymi to poprawie,
Póki co tak wg. mnie lepiej (typy z dużych, parametry z małych).

Co do opcji, jak pisałem domyślnie jest wyłączone.
Takie spowolnienie to jedynie użycie ternary operator - czyli żadne.
A tak się przydaje gzip do większych plików.

Co do tworzenia pliku -
metoda set() tworzy/aktualizuje, validate jedynie sprawdza czy plik istnieję i jest aktualny...
Więc nie rozumiem pytania.

Zmieniłem z $compress na $useGZIP ($isCompress mi nie odpowiadało, "jestKompresja" vs "użyjGZIP")....

JW. Jednak przeniosłem twoją metodę oraz pomysł na statyczne zmienne.
+ W poprzednim commicie zgodnie z komentarzem mrc skasowalem z komentarzy te typy które są zadeklarowane w metodzie - ty je znów dodałeś.
___________
Co do ".dat", zostaje przy ".txt" - jako ze to pliki tekstowe.
Jednak zmienilem ze rozszerzenie bedzie ".gz" dla plików skompresowanych niezależnie od tego co się poda w konstruktorze.
KsaR
Cytat(Pyton_000 @ 9.06.2016, 11:19:35 ) *

True false null...
Chodzi tu o dosłowne zastosowanie typu.

$var = null;

Itp.
Nie jest napisane o PHP7 oraz wymuszonych typach metod/funkcji...
A w końcu w php nie ma czegoś takiego jak:
$var = string;
$var = int;...
Pyton_000
tak ale string, int, bool to też keywordsy więc do tego bym podciągnął
kapslokk
Poza tym upper case chyba zawsze był zarezerwowany dla stałych tongue.gif
com
https://github.com/php-fig/fig-standards/bl...-style-guide.md

Php7 jest jeszcze nie zatwierdzone ale jest ;)

To co zmieniłeś nie ma sensu, bo parametr musisz podać wiec poco nam ??
Typy maja być, bo phpdoc nie jest jeszcze do php7 przygotowany i np Storm interpretuje potem twój komentarz jako klasę. Dlatego je dodałem, jak nie dajesz wgl komentarzy to wtedy można tak pisać, ale nwm czy to ma sens.

nie było $isCompress tylko $isCompressed - jest skompresowany wink.gif

Cytat
Jednak zmienilem ze rozszerzenie bedzie ".gz" dla plików skompresowanych niezależnie od tego co się poda w konstruktorze.


Może to być dla kogoś wielkim zaskoczeniem smile.gif
nospor
Mnie tez porazily te STRINGi...

  1. if ($cache->valid('number', 30)) { // cache has up to 30s ?
  2. echo $cache->get('number'); // if yes, display.
  3. }

Osobiscie preferuje gdy metoda get sama sprawdza czy jest valid czy nie i gdy nie jest valid to albo zwraca null albo rzuca wyjatek

  1. $_SERVER['REQUEST_TIME']-filemtime($path)

Chyba bardziej powinienes uzywac tim() a nie czasu z $_SERVER
com
nospor

Tylko wtedy mamy 2 odpowiedzialności wink.gif

time() biggrin.gif
nospor
tak, tak, time, szybko pisze bo jestem sledzony wink.gif

I co z tego ze dwie? Nie popadajmy w skrajnosci. Osobiscie nie widze sprawdzania za kazdym razem czy jest valid czy nie. A co jak zapomne sprawdzic i uzyje tylko get?
com
A i chodziło mi o konwencje nazywania plików wink.gif

Owszem jest to problem, no to najwyżej dostaniesz nie spójne dane smile.gif

Owszem inaczej bym to rozwiązał, żeby sprawdzał tylko raz a nie za każdym razem, ale niekoniecznie w get smile.gif
nospor
Cytat
Owszem jest to problem, no to najwyżej dostaniesz nie spójne dane smile.gif
i wlasnie temu get() powinno odwalac cala robote a nie ja mam sie jeszcze martwic. Nie lubie gdy jakies durne terorie utrudniaja mi zycie wink.gif


Ksar:
raz piszesz self::
raz static::

zdecyduj sie na jedno i uzywaj a nie mieszasz bo oczaplasu idzie dostac
KsaR
Co do lowercase keyword, argument z
https://github.com/php-fig/fig-standards/bl...-style-guide.md
Mnie przekonał i zmieniłem.

Co do time()... zanim dodałem na GH tak miałem jednak pomyślałem że rzadko kiedy skrypt się sekundę wykonuję lub więcej ale się zdarza więc poprawiłem.


Co do self va static, przeoczenie i poprawiłem na self - wg. Mnie lepiej żeby z glownej klasy pobieralo;

Co do valid w get. Coś zaraz poprawie. @edit - poprawiłem.
nospor
nie
return false
a
return null

KsaR
Cytat(nospor @ 9.06.2016, 12:57:50 ) *
nie
return false
a
return null

Dodał byś jakieś argumenty za i przeciw?

W mojej opinii:
Null = 0 = zapelnienie pustego miejsca w matematyce. ( https://pl.m.wikipedia.org/wiki/0_(liczba) )
= "Cache nie zwróciło nic".

False = fałsz = "cache jest niepoprawne", "nieaktualne".
nospor
funkcja set powinna zwraca true lub false w zaleznosci czy sie udalo czy nie.
W sumie w czasie wyjatkow powinna rzucac wyjatkiem gdy nie udalo

null vs false
jak slusznie zauwazyles false gdy jest blad. Moim zdaniem nieaktualny cache nie jest bledem a poprostu brakiem danych czyli null. Rownie dobrze moze tam jeszcze nic nie byc (null) jak i byc przeterminowane (rowniez null)

parametry konstruktora string $ext, bool $useGZIP powinny miec domyslna wartosc, skoro tak czy siak potem je ustawiasz na domyslne gdy sa null. dzieki temu wywolujac konstruktor nie musze na sile wstawiac nulli
KsaR
Cytat(nospor @ 9.06.2016, 13:17:45 ) *
funkcja set powinna zwraca true lub false w zaleznosci czy sie udalo czy nie.
W sumie w czasie wyjatkow powinna rzucac wyjatkiem gdy nie udalo

null vs false
jak slusznie zauwazyles false gdy jest blad. Moim zdaniem nieaktualny cache nie jest bledem a poprostu brakiem danych czyli null. Rownie dobrze moze tam jeszcze nic nie byc (null) jak i byc przeterminowane (rowniez null)

parametry konstruktora string $ext, bool $useGZIP powinny miec domyslna wartosc, skoro tak czy siak potem je ustawiasz na domyslne gdy sa null. dzieki temu wywolujac konstruktor nie musze na sile wstawiac nulli

Ok - zaraz poprawie na null.
Co do parametrów $useGZIP ma domyslnie false.
...i w konstruktorze jest:
  1. if ($useGZIP questionmark.gif false)
  2. # ...
  3. self::$extension = $ext questionmark.gif '.txt';

Czyli domyslnie useGZIP zostanie na false;
A $extension bedzie na '.txt' gdy się nie poda, lub ".gz" gdy się włączy kompresję.
Fred1485
Przepraszam, że się wtrącę smile.gif Dlaczego zdecydowałeś/zdecydowaliście, aby właściwości były statyczne?
nospor
Cytat
Co do parametrów $useGZIP ma domyslnie false.
Ja mowie o parametrach konstruktora

teraz masz:
public function __construct(string $path, string $ext, bool $useGZIP)
przez co wymuszasz na mnie ustawienie wszystkich trzech parametrow.

powinno byc:
public function __construct(string $path, string $ext='.txt', bool $useGZIP=false)

przez co nie musze ustawiac dwoch ostatnich jesli chce by byly domyslne

Cytat
Przepraszam, że się wtrącę smile.gif Dlaczego zdecydowałeś/zdecydowaliście, aby właściwości były statyczne?
Sluszna uwaga.Niczemu to tutaj nie sluzy bo i tak jest private
KsaR
Cytat(Fred1485 @ 9.06.2016, 13:23:51 ) *
Przepraszam, że się wtrącę smile.gif Dlaczego zdecydowałeś/zdecydowaliście, aby właściwości były statyczne?

Co prawda private (więc w dziedziczeniu nie będą dostępne)
Ale static do tego żeby należały tylko do klasy, a do obiektu już nie...
nospor
Cytat
Ale static do tego żeby należały tylko do klasy, a do obiektu już nie...
My wiemy do czego sluzy static i nie musisz nam tego wyjasniac wink.gif

Ok, prosta sytuacja:
tworzysz dwa obiekty cache:

$cache1 = new Cache('cache/', '.cache', true);
$cache2 = new Cache('cacheinnasciezka/', '.cache', true);

a teraz robisz save w cache1
$cache1->set()

hm... ciekawe gdzie sie zapisza dane z cache1 wink.gif
KsaR
Ok. Poprawiłem biggrin.gif
viking
  1. $c = new KsaR\Components\Cache('./');
  2. $b = new stdClass;
  3. $b->a = 1;
  4. $c->set('a', $b);


Dodaj jakąś serializację i zrób tam tablice i inne czyli rzeczy które się w cache trzyma na codzień. Chyba nikt nie przechowuje samych stringów. Do tego nie informujesz czy się zapis udał czy nie. Zamieniłbym też kolejność na $path, $useGzip, $ext bo teraz jeśli chcę gzipować muszę podawać rozszerzenie które w dodatku nie będzie uwzględnione.
Pyton_000
A jak chcesz zmienić ext i nie gzipować to też źle wink.gif

Użyć setterów zamiast parametrów
nospor
Jeszcze taka pierda ale strasznie drazni oko:

throw new \Exception('Path:'."\r\n".$path."\r\n".'Doesn\'t exists.');
nawaliles tych apostrofow i cudzyslowiow bez sensu...

throw new \Exception("Path:\r\n $path \r\nDoesn't exists.");
prawda ze lepiej?
Poza tym nowa linia to \n w cywilizowanym swiecie. zapomnij o windowsowych nawykach. Pozatym srodka zdania nie zaczyna sie od duzej litery i poza tym nowe linie tam sa totalnie bez sensu wiec:
throw new \Exception("Path '$path' doesn't exist.");
Jesli masz does to juz nie dajesz 's' na koncu czasownika
viking
A jak już to raczej PHP_EOL
com
Tylko że w systemie nie masz x instancji tego cache raczej, tylko jedną.

Cytat
Użyć setterów zamiast parametrów


Nie, parametry są lepsze, settery powinny zginąć. Obiekt cache to immutable, nie można zmieniać w czasie działania tego, bo można sobie narobić więcej problemów niż pożytku.
No chyba, że zrobimy tak jak w psr7 i będziemy zwracać nowy obiekt smile.gif
Dlatego też zrobiłem z tego static, ale nie testowałem i co słusznie zauważył nospor, zrobił się nam singleton.


I jak już to return; null tam jest nadmiarowy wink.gif

Na github twierdzisz ze jesteś za optymalizacja to czemu upierasz się nadal na te piramidę
  1. if (isset($path)) {
  2. if (!file_exists($path)) {
  3. throw new \Exception('Path:'."\r\n".$path."\r\n".'Doesn\'t exists.');
  4. }
  5. $this->path = $path;
  6. } else {
  7. throw new \Exception('Please specify path for cache.');
  8. }


KsaR

Widać brak Ci trochę podstaw, bo twojego konstruktora nie dało się wywołać inaczej niż z 3 parametrami, który w dodatku powinien być zgodny z typem wiec te questionmark.gif nawet nie miało sensu. Bo nawet wstawienie nulla wyrzuca błąd typu, jak już chcesz używać php7 to wypadało by trochę się z nim zaznajomić.

Dlatego chciałem go dodać w tym pull requescie i w końcu po poście Pyton_000 je tam ustawiłem.
KsaR
Cytat(com @ 9.06.2016, 21:12:53 ) *
Tylko że w systemie nie masz x instancji tego cache raczej, tylko jedną.

(...)

I jak już to return; null tam jest nadmiarowy wink.gif

Na github twierdzisz ze jesteś za optymalizacja to czemu upierasz się nadal na te piramidę
(...)
KsaR

Widać brak Ci trochę podstaw, bo twojego konstruktora nie dało się wywołać inaczej niż z 3 parametrami, który w dodatku powinien być zgodny z typem wiec te questionmark.gif nawet nie miało sensu. Bo nawet wstawienie nulla wyrzuca błąd typu, jak już chcesz używać php7 to wypadało by trochę się z nim zaznajomić.
(...)


Instancje mogą być różne...
$cacheMessages
$cacheNews
Da się oczywiście zrobić to 1 instancją ale po co ograniczać...
Rozdzielenie do różnych folderów to też „optymalizacja” zwłaszcza im więcej tam ląduje (np. 1+k)

Co do return; - zmienie

Jeśli chodzi o te if'y.
Ja wolę mieć 2 różne komunikaty (brak ścieżki, zła ścieżka) niż "brak lub zła ścieżka"
Takie coś to nie sądzę żeby bylo nawet makro-optymalizacją bo się założe że z 100* if...elseif będzie szybsze od 1. trim() czy explode()...
Takie coś to bardziej minifikacja. A tylko utrudni korzystnie wg. mnie (mniejsza o to - takie moje zdanie).

Co do parametrów. Podstawy mam raczej.
Pisałem wszystko na tablecie bez IDE typu "storm" tylko zwykłe podświetlanie składni które nawet błędów nie pokazuję, a w dodatku nie spałem. smile.gif

___
W wolnym czasie postaram się dodać to serializowanie. Zastnawiam się jak to najlepiej zrobić.
Ale gdy dodam to i tak będzie trzeba poprawić bo obstawiam że mam średni pomysł. tongue.gif
com
Cytat
Instancje mogą być różne...
$cacheMessages
$cacheNews
Da się oczywiście zrobić to 1 instancją ale po co ograniczać...
Rozdzielenie do różnych folderów to też ?optymalizacja? zwłaszcza im więcej tam ląduje (np. 1+k)


Owszem ma to sens smile.gif

Ale mój komunikat był inny wink.gif a jak tak chcesz to wydzieliłbym path i tak jak mówiłem zrobił z tego VO smile.gif

Nie chodzi o IDE, tylko o sam fakt iż nie ważne w jakim języku jak nie podasz wartości domyślnych to parametry trzeba podać, no a jak sprawdzasz ich typ to on musi się zgadzać, wiec nie przejdzie tam cokolwiek, jak w php5 smile.gif
KsaR
Ok. Dodałem tako-jako wsparcie dla tablic i obiektów.
Zmieniłem kolejność parametrów konstruktora.
W exception zmienilem "\r\n" na PHP_EOL.

Co tam jeszcze miałem?
Z tym Angielskim coś ale jak konkretnie? tongue.gif
__
+++ Ponownie zapraszam do oceny, propozycji, testów itp.
__
Ps. @com, co to jest to "VO"? Bo nie rozumiem.. zarzucisz przykładem?
__
Ps2.
set() zwraca teraz to co file_put_contents().
Nie wiem czy o to wam chodziło ;
Pyton_000
Z tą piramidą chodziło o to że zamiast:

Kod
        if (isset($path)) {
            if (!file_exists($path)) {
                throw new \Exception('Path:'.PHP_EOL.$path.PHP_EOL.'Doesn\'t exists.');
            }
            $this->path = $path;
        } else {
            throw new \Exception('Please specify path for cache.');
        }


pisze się:

Kod
if(!isset($path)) {
    throw new \Exception('Please specify path for cache.');
}

if (!file_exists($path)) {
    throw new \Exception('Path:'.PHP_EOL.$path.PHP_EOL.'Doesn\'t exists.');
}

$this->path = $path;


Prawda że czytelniej? smile.gif

Co do serializacji albo musisz serializować wszystko albo nic smile.gif
Możesz serializować tylko obiekty ale musisz wtedy użyć regexp żeby wykryć co ma być deserializowane. WP ma fajnego regexpa na to, używałem.
com
Jeśli już bardzo chcesz to typ jako stała, nigdy 0,1 wink.gif
KsaR
Cytat(com @ 10.06.2016, 12:55:45 ) *
Jeśli już bardzo chcesz to typ jako stała, nigdy 0,1 wink.gif

Pozostałość po testach biggrin.gif,
Już poprawiłem.

PPs. Co do "VO"
Czyli mam zrobić oddzielną klasę do validacji oraz zwracania ścieżki i tam zrobić coś jak w Cache "fullPath()"?
com
Ale jako parametr ma być stała przekazywana również i no ostatecznie mogło by tak też być wink.gif
LowiczakPL
Czy czasami nazwy stałych nie powinny być napisane drukowanymi literami?

http://php.net/manual/en/language.oop5.constants.php
com
Powinny smile.gif

Zgodnie z PSR, bo php dopuszcza, bo to tylko konwencja smile.gif
LowiczakPL
Dodatkowo dałbym strategię dla typów kesza, co zlikwidowało by IF-y,

zamiast liczbowej nazwy typu przekazywał bym nazwę strategii.
com
Owszem można by zrobić fabrykę nawet wink.gif

I nie żadnego elseif

  1. if (!isset($path)) {
  2. throw new \InvalidArgumentException('Please specify path for cache.');
  3. } elseif (!file_exists($path)) {
  4. throw new \DomainException('Path:'.PHP_EOL."„{$path}”".PHP_EOL.'Does not exists.');
  5. }


Dlaczego jesteś taki uparty, skoro Pyton_000 napisał tak
  1. if(!isset($path)) {
  2. throw new \Exception('Please specify path for cache.');
  3. }
  4.  
  5. if (!file_exists($path)) {
  6. throw new \Exception('Path:'.PHP_EOL.$path.PHP_EOL.'Doesn\'t exists.');
  7. }
  8.  
  9. $this->path = $path;


I co to wgl za „ ” są ohmy.gif

pomijając to że nie stosujesz się do tego co pisał nospor apropo poprawności komunikatu wink.gif

a sam path jako byt, to nie Domain wink.gif
KsaR
Cytat(com @ 11.06.2016, 19:31:33 ) *
Owszem można by zrobić fabrykę nawet wink.gif

I nie żadnego elseif

(...)

I co to wgl za „ ” są ohmy.gif

pomijając to że nie stosujesz się do tego co pisał nospor apropo poprawności komunikatu wink.gif

a sam path jako byt, to nie Domain wink.gif

* Co do strategy, factory - nie widzę takiej potrzeby dla takiej mikro-klasy.

* Poprawie.

* Cudzysłowia. biggrin.gif

* Jak mam się stosować do czegoś czego nie rozumiem ?

* Zmienie na Exception. ;d
com
chodzi o to, że tak jest poprawnie:
throw new \Exception("Path '$path' doesn't exist.");

No albo się pisze czysty kod albo ma byle polska nie zginęła tongue.gif

Nie stosował bym egzotycznych znaczków bo rożnie mogą to przeglądarki interpretować, poza tym
to tylko błąd, poco jakieś eksperymenty bo można smile.gif

Co do stałych jeszcze bym się zastanowił czy ten zwrot jako jest nam potrzebny smile.gif
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2024 Invision Power Services, Inc.