Prosím o „zkontrolování“ kódu

Upozornění: Tohle vlákno je hodně staré a informace nemusí být platné pro současné Nette.
Budry
Člen | 88
+
0
-

Zdravím,
doufám že to není moc troufalé, ale dokončil jsem svojí první aplikaci psanou v Nette a abych se přiznal stále mám pocit že jsem spoustu věcí kolem Nette nepochopil nebo pochopil špatně. Rád bych se Nette věnoval i nadále a psal v něm aplikace které by měli nějakou úroveň a nemusel bych se za svůj kód stydět.
Napsat aplikaci která zdánlivě funguje není až takový problém, problém je napsat jí správně, tak abych se nedopouštěl zbytečných chyb v kódu.
Chybí mi možnost nechat kód zkontrolovat někým kdo tomu rozumí více než já a opravit takové ty „drobnosti“ co se snadno do kódu nasekají.
Je mi jasné že luštit cizí kód se nikomu asi chtít nebude, ale rád bych se dokopal do stavu kdy se za kód stydět nebudu muset, čímž si teď zrovna jistý nejsem.

Kód jsem vystrčil vedle aby nebyl post zbytečně dlouhý

HomepagePresenter

Homepage_model

AuthPresenter

Auth_model

Předem díky všem co se obětují a podívají se na to :)

Lopata
Člen | 139
+
0
-

Tak zběžně:

  1. Sjednoť si coding standard. Homepage_model a HomepagePresenter… Dále (alespoň mně to tak připadá) je do očí bijící, jak kombinuješ dva jazyky. Buď getRecipe(), nebo vratRecept().
  2. Metoda getRecept je napsaná a používaná strašlivě. Takhle se vykoná mnoho zbytečných dotazů. Neboj se si do šablony jednoduše dát pole:
<?php
// model
public function getRecept($id) {
    return dibi::query('SELECT * FROM [recepty] WHERE id_receptu = %i', $id)->fetchAll();
}
// presenter
$this->template->recept = $this->homepage_model->getRecept($id);
?>
  1. Metoda deleteRecept() si říká o transakci…
  2. Metoda sendRecept(): HTML v modelu? Do šablony s tím!
  3. Nastuduj si přihlašování uživatelů
  4. Nette mail umí šablony.

Závěr: Pročti si dokumentaci. Je mladá, krásná a bohatá… Dále doporučuji trochu víc nastudovat samotné PHP. Např. kniha „1001 tipů a triků pro PHP“ od Jakuba Vrány shodou pojednává o mnoho chybách, které jsi tam udělal. Např.

<?php
$result = dibi::query('SELECT * FROM [users] WHERE %and', $dbData);
        if(count($result))
        {
            unset($result);
            return true;
        }
        else
        {
            unset($result);
            return false;
        }
?>

by se určitě dalo trochu zlepšit… ;-)

22
Člen | 1478
+
0
-

no ono je tam spousta zbytečností, např:

//original
    public function getSuroviny($id)
    {
        $result = dibi::query('SELECT
                                    [suroviny.mnozstvi],
                                    [surovina.nazev] AS surovina,
                                    [jednotka.nazev] AS jednotka
                                FROM
                                    [suroviny]
                                LEFT JOIN
                                    [surovina] ON [surovina.id_surovina] = [suroviny.id_surovina]
                                LEFT JOIN
                                    [jednotka] ON [jednotka.id_jednotka] = [suroviny.id_jednotka]
                                WHERE
                                    [id_receptu] = %i', $id);

        $all = $result->fetchAll();
        return $all;
    }

//úprava
    public function getSuroviny($id)
    {
        return dibi::fetchAll('
		SELECT [suroviny.mnozstvi], [surovina.nazev] AS surovina, [jednotka.nazev] AS jednotka
		FROM [suroviny]
		LEFT JOIN [surovina] ON [surovina.id_surovina] = [suroviny.id_surovina]
                LEFT JOIN [jednotka] ON [jednotka.id_jednotka] = [suroviny.id_jednotka]
                WHERE [id_receptu] = %i', $id
	);
    }
Filip Procházka
Moderator | 4668
+
0
-

Pokaždé když napíšeš HTML do modelu, tak bůh zabije koťátko.

22
Člen | 1478
+
0
-

hehe, piš HTML do modelu :-)

Budry
Člen | 88
+
0
-

Lopata:

  1. Jazyky opravím, moje chyba
  2. Přišlo mi jednoduší usnadnit práci šabloně, pohraji si s tím ještě
  3. Dostuduji a opravím, koukám že co se týče sql dotazů sem na tom bídně, o transakci slyším prvně..
  4. O tom že email funkce mail() umí poslat i šablonu jsem četl, ale nějak jsem byl v tomto směru skeptický. Opravím
  5. Kouknu na to
  6. viz. 4)
  7. Knihu jsem letmo pročetl (víc jsem zatím nestihl) zkusím se tím prokousat, ale je fakt že by zápis
return (count($result)) ? true : false;

měl stačit (i když nevím jestli jsem tím něco vyřešil…)

22:

Projdu si to, s dibi jsem se setkal poprvé a nevím co vyjde na použití lépe jestli dibi nebo přímo Nette\Database.
U dibi postrádám nějaký přehled všech funkcí a možností jejich quickstart obsahuje málo ukázek a jediná dokumentace je API a to my nepřijde jako moc dobrá volba, volil bych spíše přehlednější dokumentaci jednotlivých možných příkazů a funkcí…(ale to jen naokraj)

22
Člen | 1478
+
0
-

Tak to si Nette\Database nepomůžeš. V dibi distribuci jsou i nejake examples například…jinak tady je téměř všechno, co potřebuješ pro začátek, zbytek na foru dibi nebo tady, co se týká DI.

joe
Člen | 313
+
0
-

@Budry:

  • chybí autentikátor, zjišťování přihlášeného uživatele pak v presenteru $this->user->isLoggedIn()
  • sjednocení názvů, to už tu někdo psal
  • $this->template->nadpis = $this->homepage_model->getRecept($id, 'nadpis'); a podobné nahradit $this->template->recept = $this->homepage_model->getRecept(); a v šabloně pak volat $recept->nadpis apod.
  • místo vracení v čísel v modelech (regUser()) je dobré používat vyjímky
  • v HomepagePresenteru pokud nastane chyba v public function actionDelete($id), tak nedojde k přesměrování
  • metoda deleteRecept($id_receptu) je zbytečná, jednoduše by stačilo
dibi::query('DELETE FROM [recepty] WHERE [id_receptu] = %i', $id_receptu);
	return dibi::affectedRows() ? TRUE : FALSE;

zbytek se dá zařídit přímo v databázi přes relace (v případě MySQL a InnoDB)