Weblabor

Bocsi, de szerintem

- működő oldal, ami ref. lehet;
- nem derül ki, mennyire saját fejlesztés, mivel csináltad;
- a lábléc agyonvágja az amúgy sem tetszetős designt (Figyelem! Ez erősen szubjektív, csak a véleményem tükrözi, egy designer véleményére inkább adj!);
- a menüszerkezet kissé fura, 2 főmenü, millió "almenü", amik inkább másodlagos menük;
- az oldalon használt captcha sem tetszik nekem;
- látva a forrást, hát szeretsz szkripteket ide-oda is behányni, emellett csak névleg használod a HTML5-öt, a tag-eket nem, ez mondjuk nálam nem feltétlen hátrány, de másnak jelentheti azt, hogy gőzöd sincs a HTML5-ről.

Javaslat:

- fejezd be és működtesd az oldalt, ha működik (=használják), az referencia;
- keress egy grafikus havert, akivel ketten kipofozzátok szépre;
- írj ki rendes copyright-ot, ha van grafikus haver, az ő nevét is.

Így még - szerintem - lehet belőle referencia, főleg ha saját programozás, nem valami CMS.

Nagyon remélem, hogy építő kritikának veszed - ennek szántam, véletlenül sem bántónak.

Előrebocsájtom, hogy elég fáradt vagyok, úgyhogy csak "szúrópróba", ameddig tetszik is.

Adattábla:
- én hiányolom: UNIQUE KEY `u_email` (`u_email`), mert nem szokás ugyanazt az e-mailt sem kétszer engedni;
- szoktam belépési időt, és előző belépési időt is tárolni (datetime), így lehet timelimit-et is belőni, ha "nem mozog" + WL-hez hasonlóan friss tartalmakat, stb.; IP címet azért, hogy kiléptessem, ha változik - szóval jócskán nagyobb nekem a "users_table";
- valamilyen plusz oszlopot (text) még mindenképp tennék hozzá, amibe esetleg szerializált egyéb adatokat lehet tenni, vagy jóval több oszlopot, mert hátha egyszer címet is akarsz kérdezni, stb., aztán a regelős formot úgy variálod, ahogy akarod;
- az `u_reg_date` date DEFAULT NOW(), egyszerűsítené a dolgod, és miért nem datetime?;
- sót már fentebb talán írtam, hogy felesleges külön oszlopba gyártani hozzá, darabolhatsz pl. a `u_nickname`-ből, vagy felhasználhatod egy meghatározott rendben az összes NOT NULL oszlopot, stb.;
- nincs megadva táblatípus, miért? (a configok közül kihagytad a database.php-t, így az sem derül ki, milyen drivert használsz, így nem lesz rendesen értékelhető a model).
- Még két táblát hiányolok a jogosultságkezeléshez, és egyet vagy kettőt naplózáshoz és statisztikához.

Controller:
- Mivel a controllerek "controllers", a modellek "models" könyvtárban vannak, teljesen felesleges a fájlnévbe is beleírni. (Persze azonos nevű nem lehet controller és model, de a controller lehet "tagok" is, és nem kell agyonroutingolni, hogy magyar legyen...)
- Én (és a CI is) az osztályok legelején deklarálok változókat, a __construct() előtt, valamint a tabulátorok még mindig nem teljesek (nekem).
- A form_validation osztálynak is kényelmesen meg lehet adni config fájlban a beállításait, így hatalmas a controller, többször leírod ugyanazt, és rosszul hordozható.
- Az ilyen kód kerülendő a controller-függvényekben, ki kell emelni külön függvénybe:Egy private fv, amit paraméterekkel hívsz meg, esetleg a hívó fv-ben a Template Parser Class segítségével állítod elő a fő tartalmat. Így könnyeben megcsinálod majd a mobil kinézetet is.
- Elfelejtett jelszó: mit keres a controllerben HTML kód? Szintén Template Parser Class. Nem mondom, hogy nekem sosincs ott HTML, de max. ennyi: $this->data['message'] .= '<p>Az XXX funkciókhoz be kell jelentkezned.</p>';. (Természetesen ezt a data tömböt kapja a végén a view réteget hívó fv., és kismillió paraméter van belegyűjtve, l. mobilnézet cikkem, az az alap.) Itt is leírod ugyanazokat a configokat, mint a bejelentkezésnél. Ha most hirtelen meg kéne oldani, hogy min. 6 karakter legyen a felhasználónév, végig kéne nyálazni az egész controllert, nem pedig egy config fájl egy sorát módosítani (esetleg két sorát, de akkor már nem elég jól gondolkodtál).
Milyen tárgy ez egy e-mailnek?! (122. sor)
Nincs az e-mailnek alternatív szövege, így a felhasználó, ha csak plain/text e-mailt fogad, nem kap semmit...
- password_reset: adatbázist csak modelből bűvészkedjük (156. sor), és a sózás művelete a model egy fv-ének kellene lennie. A sózást is vagy 20x leírod, ha változtatni akarsz rajta, szinte biztos bukta, és spagetti a kód ettől a sok ismétléstől.
Kicsit bonyolultabb model, és sokkal egyszerűbb controller.
- Regisztrálás: ugyanazok a config- és e-mail hibák, mint fentebb.
- Közben meg van egy _mailer($content, $recipient, $subject) privát fv., ami némi átalakítással használható lenne fentiek helyett, vagy egy ehhez hasonló. De ebből is hiányzik az alt text.
- Logout: mivel a táblában semmi erre vonatkozó adat nincs, elegendő a sessiont "kiirtani", de ha látogatottságot is számolsz, akkor csak az adott session-változókat kéne törölni, nem az egészet.
- A birth_check() és a verify($name, $key) miért publikus fv? URL-ből hívható?! Egyébként a modelben a helyük, vagy privát.
- unique_check: az első ellenőrzés miatt tiltanod kell felh. névben a @ karaktert... Ez is miért public? (Jaj, lehet, hogy a form_validation-nek csak úgy jó, akkor viszont a routs-ban le kell tiltani.)
- 357. sor: elég furcsa nevet adtál a session-ben a captcha-nak... Remélem ezt nem gépelted el sehol...
- 379. sorban biztos, hogy 'fwhere' van?

Na, mostanra ennyire futotta, bocsi, de modelre már nem. A controllerből is látszik, hogy nemigazán vannak helyén a dolgok, tulajdonképpen bármi adatot előállítani is a modelben szoktam (nagyon kevés kivétellel), a controller tényleg csak vezérli az eseményeket, alapvető dolgokat ellenőriz, de már ezt is modell(ek) segítségével. Hogy átlátható legyen a vezérlő.
Modelleknél is meghatározott rend uralkodik, mi az a szint, ami felett már kiszervezem a feladatot privát fv-be, vagy írok egy saját osztályt rá.

Szerintem ezeken gondolkodj el, attól valószínű megváltozik erősen a model is, majd talán még egyszer meg nézem, de nem ígérem biztosra.

Biztonsági téren még van mit javítani rajta. Template változók nincsenek escapelve, bejelentkezés után nem váltasz session id-t, nem paraméteres lekéredezések, adatbázisnak nincs megadva a kódolása... a sha1 helyett is lehetne elegánsabb hash fügvényt találni.