[www] [skrypt] biblioteka filmów dla programu XBMC Opcje

[Regss]

Witam! Napisałem swój pierwszy pełny skrypt pomijając oczywiście wcześniejsze zabawy w pisanie krótkich kodów php na 50 linijek. Próbowałem stosować się do wszystkich standardów php, html, css.

Skrypt skierowany jest dla posiadaczy domowego kina multimedialnego opartego o program XBMC i posiadanej bazy filmów. Ma on za zadanie parsować pliki xml dostarczone z tegoż programu wraz z grafiką, uporządkować je w bazie danych i wyświetlić ładnie wyglądającą prezentację.

Proszę Was o uwagi odnośnie samego skryptu bo to jest dla mnie najważniejsze aby na początku oduczyć się złych nawyków i zdobywać poprawną wiedzę w pisaniu w php. Oczywiście na temat wyglądu uwagi też mile widziane.

strona główna:
http://regss.sk1.pl/test/index.php

strona panelu administratora:
http://regss.sk1.pl/test/panel.php

hasło do panelu: test

Śmiało można usuwać, dodawać, importować. (celowo na potrzeby testów wyłączyłem usuwanie plików po udanym imporcie).

A oto kod źródłowy:

google code

paczka z kodem: ZIP

Pozdrawiam.

Ten post edytował Regss 29.12.2011, 22:29:16

[Shili]

1) config.php wygląda jak WYMAGANY plik (w końcu bez configu raczej strona nie pójdzie). Czemu to jest dołączane za pomocą include a nie require?

2) function.php to nie jest dobra nazwa

3) Gdybyś skorzystał z np. Doxygena do komentarzy, to miałbyś już dokumentację projektu.

login.php

[PHP] pobierz, plaintext 
    unset($_SESSION['id']);
    session_destroy();
[PHP] pobierz, plaintext 

Pierwsze unset jest zbędne.

[PHP] pobierz, plaintext 
// Logout
if (isset($_GET['logout'])) {
    unset($_SESSION['id']);
    session_destroy();
    header('location:index.php');
}
 
// Login panel
if (!isset($_POST['pass'])) {
    $content_output = '<form action="login.php" method="POST">' . $lang['l_panel_pass'] . ':<br/><input id="l_pass" name="pass" type="password" /></form>';
} else {
    if ($_POST['pass'] === $panel_pass) {
        $_SESSION['id'] = md5($_SERVER['REMOTE_ADDR']) . md5($panel_pass);
        header('location: panel.php');
    } else {
        $content_output = $lang['l_panel_wrong'] . '. <a href="login.php">' . $lang['l_panel_again'] . '</a>';
    }
}
[PHP] pobierz, plaintext 

W bloku login panel dałabym elseif - aktualnie sprawdza isset($_GET['logout']), a potem zawsze sprawdza !isset($_POST['pass']), a podejrzewam, że te dwie rzeczy wykluczają się.

config.php
Zabrakło mi zmiennej configuracyjnej ustalającej port do bazy. Jasne, można wepchnąć w host, natomiast lepiej IMO byłoby rozdzielić.

[PHP] pobierz, plaintext 
$perPage = 30; // Movies per page, If you do not want to have pagination, type a larger number than the number of movies
[PHP] pobierz, plaintext 

If you do not want to have a pagination type 0 - standardowo 0 znosi wszelkie limity

[PHP] pobierz, plaintext 
 * Don't edit nothing below this line!#
[PHP] pobierz, plaintext 

Do not edit anything...
Podwójne zaprzeczenie nie istnieje w angielskim (IMG:style_emoticons/default/smile.gif)

panel.php

[PHP] pobierz, plaintext 
    if ($_GET['option'] == 'del_all') {
        $content_output = delete_table($table_name, $lang);
    }
    if ($_GET['option'] == 'create_new') {
        $content_output = create_table($table_name, $lang);
    }
    if ($_GET['option'] == 'xml_file_info') {
        $content_output = xml_file_info($xml_file, $lang, $detect_encoding);
    }
    if ($_GET['option'] == 'nfo_file_info') {
        $content_output = nfo_file_info($table_name, $lang, $detect_encoding);
    }
    if ($_GET['option'] == 'xml_import') {
        $content_output = import_xml($xml_file, $table_name, $lang, $detect_encoding);
    }
    if ($_GET['option'] == 'clear_cache') {
        $content_output = clear_cache($lang);
    }
[PHP] pobierz, plaintext 

Aż się prosi o switch zamiast if.
Nie mówiąc już o tym, że chyba bardzo nie lubisz else. Jeśli $_GET['option'] będzie równe del_all, to skrypt zrobi sprawdzenie wszystkiego, mimo że dopasował się do pierwszego warunku. A warunki są rozłączne.

[PHP] pobierz, plaintext 
  $sql_movies = 'SELECT id FROM ' . $table_name;
    $result_movies = mysql_query($sql_movies);
    $table_rows = '<span class="green">' . mysql_num_rows($result_movies) . '</span>';
[PHP] pobierz, plaintext 

SELECT COUNT(id)
mysql_num_rows jest przydatne wtedy, gdy odpalasz normalne zapytanie i dodatkowo chcesz policzyć ile elementów zwróciło;
Jeśli zależy Ci tylko na liczbie - COUNT jest bardziej wydajne.

index.php

[PHP] pobierz, plaintext 
if ((!mysql_query('SELECT id FROM ' . $table_name . ' LIMIT 1')) or (mysql_num_rows(mysql_query('SELECT id FROM ' . $table_name . ' LIMIT 1')) < 1)) {
        header('Location: panel.php');
}
 
// sets the variables
if (!isset($_GET['id'])) {
    $id_result = mysql_query('SELECT id FROM ' . $table_name . ' LIMIT 1');
[PHP] pobierz, plaintext 

Trzy razy wykona się pobieranie toczka w toczkę tego samego zapytania.
Co prawda podejrzewam, że danych w bazie będzie bardzo niewiele, natomiast jest to zły nawyk.
Zrób mysql_query raz przed tymi wszystkimi ifami.

Zastanawiam się również czy koniecznie potrzebujesz regexpów w zapytaniach:

[PHP] pobierz, plaintext 
$genre_mysql = $val;
[PHP] pobierz, plaintext 

Gdzie val wydaje się pochodzić z zapytania z tabeli dotyczącej gatunków filmów. Na szybki rzut oka nie ma tam nic dotyczącego konieczności użycia regexpa. Like mysqlowy, mimo że tragicznie wolny jest dużo szybszy od regexpów.

To tak ogólnie. Na pierwszy rzut oka. Mam nadzieję, że co nieco Ci się z tego przyda.