Názor na DB layer na zakázku

Je to akorát OOP obálka nad mysql_query fcemi.
1) pouzita spatna escapovaci fce - patri mysql_real_escape_string
2) SQL injection na $id - nikde neni osetreno, primo se vlozi
3) mysql_query rodina fci je uz dnes zastarala, pouzil bych PDO a vyuzil vazani promennych
4) popremyslel bych nad systemem ruznych DB driveru (MySQL, Sqlite apod.)
5) technicky bych zvolil staticky registr neco jako db::query()...

6) jsem zastance svych reseni, ale v tomto pripade si tam dejte dibi

Lamicz:
mysql_escape_string

Fisak:
* Asi budu za formalistku, ale nelíbí se mi ten kód, špatně se mi čte. Např. několik různých velikostí odsazení uvnitř jedné třídy. Chybí phpdoc. Nejednotný styl. Mnohdy nevhodné či nepřesné pojmenovávání proměnných a metod. Konkrétněji:

* K čemu je metoda DatabaseException::__const - nikde není volaná, tzn. asi to měl být konstruktor. V tom případě je ale zase zbytečný, nedělá nic navíc oproti konstruktoru rodiče.
* třída SQLBase: konstanty jako výchozí hodnoty omezují zapouzdření.
* Metoda getConnection má podle názvu vracet (instanci?) DB připojení, ve skutečnosti připojení navazuje.
* Parametry konstruktoru bych dala spíš jako asoc.pole, je to jednodušší než pamatovat si požadované pořadí. Nerozumím té logice, že můžu konstruktoru předat buď pole přihlašovacích údajů, anebo jen řetězec s názvem DB.
* Proč metoda Query vrací výsledek mysql_query? Porušuje tím zapouzdření; čekala bych buď logickou hodnotu, nebo fluent interface.
* třída DatabaseFunction: tady by se vůbec neměly vyskytovat nízkoúrovňové funkce typu mysql_escape_string či mysql_fetch_assoc, ty patří do SQLBase.
* Chybí escapování názvů sloupců a tabulek.
* Není mi moc jasné, k čemu je členská proměnná ID, zápis $this->ID[$this->table] působí dost divně.
* Cyklus na ř.33-36 lze sloučit s předchozím (od ř.25), MySQL příkaz INSERT rovněž podporuje syntax ... SET col = 'value'.
* Metoda ReadJoinTable: co když chci řadit data podle sloupce z připojené tabulky?
* foreach($join as $join=>$using) kupodivu funguje, ale vypadá to dost nestandardně a navíc za cyklem bude v proměnné $join ne pole, ale poslední jeho klíč.
* Metoda getCount je neefektivní, existuje SELECT COUNT(*) FROM ...
* Metoda Delete: použití if($this->Query(... nedává smysl, metoda Query buď vrací false, nebo háže výjimku, má tam teda místo podmínky být blok try-catch.
* Metoda getID: if(mysql_escape_string($where)){ - nechápu smysl.
+ souhlasím s Lamiczem v bodech 2, 4, 6

Fisak:
„Nechal sem si naprogramovat od kamaráda Db layer“
„nad dibi sem přemýšlel ale mám radši něco svého“

Mně se zdá, že si protiřečíš. V ani jednom případě to nebude tvůj výtvor.

Fisak:
Jednoduše řečeno, celé zle. Zkusím shrnout výhrady (možná se budou překrývat názory Lamicze a Tori.
• Strukturování kódu je katastrofální, chybí jakákoliv rozumná štábní kultura.
• Kód není nijak komentovaný. Už jen proto bych měl odpor jej vůbec používat (tedy za předpokladu, že bych vůbec chtěl).
• Proč má třída výjimky DatabaseException vlastnost $message? Je to pouze redundantní informace (tutéž nese rodič). Potřeba __toString u výjimky je přinejmenším k zamyšlení (výjimky se standardně na string nepřevádějí).
• Ke konceptu třídy SQLBase:
- Dělá věci, které nemá.
- Rozhodně by za žádných okolností neměla sama hledat údaje k přístupu do databáze, naopak je má dostat. A to ani defaultní.
- Parametr konstruktoru je nicneříkající a zavádějící.
- Když už mermomocí má tahle třída existovat, proč není abstraktní?
• Třída DatabaseFunction je o něco úsměvnější:
- Prvně, jaký je vůbec její význam? Z názvu to rozhodně nevyplývá.
- Argumenty konstruktoru jsou nicneříkající.
- Pokud správně vidím, třída se tváří jako reprezentace tabulky. Proč je tedy později $IDField používáno jako registr všech tabulek?
- Chybí podpora kompozitních klíčů.
- K těm dalším metodám jen obecně, ani jedna z nich neprovádí nic co by bylo generalizovatelné na jakoukoliv tabulku. Jejich chování je nejasné a nepředvídatelné.
• Používat prehistorickou ext/mysql je v dnešní době poněkud mimo trend. Už i v dokumentaci najdeš pod každou mysql_* funkcí toto: „Use of this extension is discouraged. Instead, the MySQLi or PDO_MySQL extension should be used. See also MySQL: choosing an API for more information.“

Hlavně prosím tento výtvor nenazývej DB layerem, jelikož jím opravdu není.

„no nad dibi sem přemýšlel ale mám radši něco svého...“
To je hloupý argument, zejména když něco svého není ani zdaleka tak kvalitní jako např. zmíněná dibi.


Tori:
„mysql_escape_string“
„This function has been DEPRECATED as of PHP 5.3.0. Relying on this feature is highly discouraged.“ Deprecated (bez warningu) je dokonce už od PHP 4.3.0.

PS: Kód jako odkaz na Ulož.to se mi nelíbí, navrhuji jeho přídání do [#1].