Czy taki manager baz jest dobry? Opcje

[Utilaft]

Witam. Uczę się z eduweb php oop. I jest taka klasa obsługująca bazę danych:

[PHP] pobierz, plaintext 
<?php
 
 
class DatabaseManager {
 
    static public function getConnection() {
 
        $conn = @new mysqli(DB_SERVER, DB_USERNAME, DB_PW, DB_DB);
 
        if(mysqli_connect_errno()) {
            $conn_errno = mysqli_connect_errno();
            $conn_error = mysqli_connect_error();
 
            // W przypadku wystąpienia problemów z połączeniem, zapisz informację w pliku LOG i zakończ działanie skryptu
           LogFile::AddLog("Nastapił błąd połączenia numer [ $conn_errno ] z bazą danych:  o treści: [ $conn_error ]", __LINE__, __FILE__);
           exit();
 
        } else {
 
            $conn->query("SET NAMES 'utf8'");
            return $conn;
 
        }
 
    }
 
    static public function selectBySQL($SQL) {
 
        $conn = self::getConnection();
        //$SQL = $conn->real_escape_string($SQL);
        $result = $conn->query($SQL);
 
        if(!$result) {
 
            LogFile::AddLog("Wystąpił błąd połączenia z bazą danych!", __LINE__, __FILE__);
            return false;
 
        } else {
 
            $resultArray = Array();
 
                while(($row = $result->fetch_array(MYSQLI_ASSOC)) !== NULL) {
 
                    $resultArray[] = $row;
                }
 
        }
 
        if(count($resultArray) > 0) {
                return $resultArray;
            } else {
                LogFile::AddLog("Zapytanie bazodanowe zwróciło pusty wynik!", __LINE__, __FILE__);
                return false;
            }
 
        mysqli_close($conn);
 
    }
 
    static public function selectData($TABLE, $COLUMNS = Array("*"), $WHERE = Array(), $LOGIC_OPER = "=", $OPER = "AND") {
 
        $conn = self::getConnection();
 
        $SQL = "SELECT ";
 
        if(count($COLUMNS) == 1) {
            $SQL .= $COLUMNS[0];
        } else {
            foreach($COLUMNS as $column) {
                $SQL .= $column.",";
            }
        }
 
        $SQL = rtrim($SQL, ',');
 
        $SQL .= " FROM {$TABLE}";
 
        if(count($WHERE) > 0) {
 
            $SQL .= " WHERE ";
 
            foreach($WHERE as $key => $val) {
                $SQL .= $key.$LOGIC_OPER."'".$val."' ".$OPER." ";
            }
 
            $SQL = substr($SQL, 0, strlen($SQL)-(strlen($OPER)+2));
 
        }
 
        $result = $conn->query($SQL);
 
        if(!$result) {
 
            LogFile::AddLog("Wystąpił błąd połączenia z bazą danych!", __LINE__, __FILE__);
            return false;
 
        } else {
 
            $resultArray = Array();
 
                while(($row = $result->fetch_array(MYSQLI_ASSOC)) !== NULL) {
 
                    $resultArray[] = $row;
                }
 
        }
 
        if(count($resultArray) > 0) {
                return $resultArray;
            } else {
                LogFile::AddLog("Zapytanie bazodanowe zwróciło pusty wynik!", __LINE__, __FILE__);
                return false;
            }
 
        mysqli_close($conn);
 
    }
 
    static public function updateTable($TABLE, $SET, $WHERE = Array(), $OPER = "AND") {
 
        $conn = self::getConnection();
 
        $SQL = "UPDATE {$TABLE} SET ";
 
        foreach($SET as $key => $val) {
            $SQL .= $key."='".$val."',";
        }
 
        $SQL = rtrim($SQL, ',');
 
        if(count($WHERE) > 0) {
 
            $SQL .= " WHERE ";
 
            foreach($WHERE as $key => $val) {
                $SQL .= $key."='".$val."' ".$OPER." ";
            }
 
            $SQL = substr($SQL, 0, strlen($SQL)-(strlen($OPER)+2));
 
        }
 
        $result = $conn->query($SQL);
 
        if($result) {
                return true;
            } else {
                LogFile::AddLog("Zapytanie bazodanowe UPDATE nie wykonało się poprawnie!", __LINE__, __FILE__);
                return false;
            }
 
        mysqli_close($conn);
 
    }
 
    static public function deleteFrom($TABLE, $WHERE = Array(), $OPER = "AND") {
 
        $conn = self::getConnection();
 
        $SQL = "DELETE FROM {$TABLE}";
 
        if(count($WHERE) > 0) {
 
            $SQL .= " WHERE ";
 
            foreach($WHERE as $key => $val) {
 
                $SQL .= $key."='".$val."' ".$OPER." ";
 
            }
 
            $SQL = substr($SQL, 0, strlen($SQL) - (strlen($OPER)+2));
 
        }
 
        $result = $conn->query($SQL);
        if(!($result)) {
            LogFile::AddLog("Usunięcie elementu bazy danych było nie możliwe do zrealizowania.", __LINE__, __FILE__);
            return false;
        } else {
            return true;
        }
 
        mysqli_close($conn);
 
    }
 
    static public function insertInto($TABLE, $DATA) {
 
        $conn = self::getConnection();
 
        $SQL = "INSERT INTO {$TABLE}";
        $SQL .= " (";
 
        foreach($DATA as $key => $val) {
            $SQL .= $key.",";
        }
 
        $SQL = rtrim($SQL, ",");
        $SQL .= ") ";
        $SQL .= "VALUES";
        $SQL .= " (";
 
        foreach($DATA as $val) {
            $SQL .= "'".$val."',";
        }
 
        $SQL = rtrim($SQL, ',');
        $SQL .= ")";
 
        $result = $conn->query($SQL);
        if(!($result)) {
            LogFile::AddLog("Wstawienie nowego elementu do bazy danych zakończyło się niepowodzeniem.", __LINE__, __FILE__);
            return false;
        } else {
            return true;
        }
 
        mysqli_close($conn);
 
    }
 
}
 
?>
[PHP] pobierz, plaintext 

takie przykładowe zapytanie:

DatabaseManager::selectBySQL("bla");

Dodawanie rekordów też podobnie typu tablica. W kodzie wszystko macie.

Oraz config z automatycznym ładowaniem klas.

[PHP] pobierz, plaintext 
<?php
ob_start();
 
// OKREŚLENIE POŁOŻENIA STRONY W SERWISIE - DEFINICJA <BASE ... />
$AbsoluteURL = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') ? 'https://' : 'http://';
$dirCat = dirname($_SERVER['PHP_SELF']);
$AbsoluteURL .= $_SERVER['HTTP_HOST'];
$AbsoluteURL .= $dirCat != '\\' ? $dirCat : "";
$slash = substr($AbsoluteURL, -1);
 
$NewURL = $slash != '/' ? $AbsoluteURL.'/' : $AbsoluteURL;
 
 
// STAŁE DLA BAZY DANYCH
define('DB_SERVER', 'localhost');
define('DB_USERNAME', 'eduweb');
define('DB_PW', 'eduweb2011');
define('DB_DB', 'supercms');
 
// STAŁA DLA ADRESU I LOKALIZACJI APLIKACJI
define('SERVER_ADDRESS', $NewURL);
 
// STAŁA DLA LOKALIZACJI KATALOGÓW I PLIKÓW
define('LogFolder', 'LOG', true);
 
set_include_path(get_include_path(). PATH_SEPARATOR . "CLASS");
set_include_path(get_include_path(). PATH_SEPARATOR . "CLASS/Managers");
set_include_path(get_include_path(). PATH_SEPARATOR . "LIBRAY");
 
// Magiczna funkcja automatycznie ładująca klasy wg. zapotrzebowania
 
function __autoload($className) {
 
    @include_once($className.".class.php");
 
}
 
?>
[PHP] pobierz, plaintext 

Czy takie rozwiązanie jest dobre? Chciałbym używać tego, jako podstawy w moim przyszłych projektach.

[Fifi209]

Odpowiadam na pytanie: nie.

Mogę podać kilka powodów:

• Nie do końca wiesz do czego służa funkcje statyczne - po prostu walisz i już
• Czemu nie użyjesz PDO i wyjątków? Skoro ma być obiektowo...
• Nie sprawdzasz w funkcjach czy masz połączenie z bazą, ew. wyświetlasz komunikat, że coś z bazą nie tak
• Zwracasz z wielu funkcji false w przypadku niepowodzenia a komunikat zapisujesz w logach
• Używasz stałych w celu połączenia z bazą zamiast przekazywać to przez konstruktor
• Brak możliwości połączenia z kilkoma bazami naraz
• Używasz __autoload nie wiedząc jak to ma działać, tylko dodajesz od razu ścieżki do include

Mogę mnożyć, ale zastanów się najpierw nad tym co napisałem

Ten post edytował Fifi209 13.03.2013, 00:22:02

[Adi32]

[*]Używasz stałych w celu połączenia z bazą zamiast przekazywać to przez konstruktor

ManagerDB to najczęściej singleton, wtedy przekazywanie przez konstruktor odpada.
Jednak lepiej byłoby przekazywać te stałe globalną typu $config['db_default']['host']...

Ten post edytował Adi32 13.03.2013, 08:28:24

[Utilaft]

Myślałem ze ten kod będzie dobry, bo w końcu był nagrywany przez profesjonalistów (chyba?)

No tak, nie do końca jeszcze widzę sens stosowania metod statycznych.

Chciałem na tym kodzie się uczyć, mieć taką podstawę.

[nospor]

Niby profesjonaliści a takie coś robią:
LogFile::AddLog("Zapytanie bazodanowe zwróciło pusty wynik!", __LINE__, __FILE__);

Super, fajnie, że mnie poinformowali, że zapytanie zwróciło pusty wynik.... tylko kurcze....jakie zapytanie? No i jeszcze ta linia i plik.... a na grzyba mi linia i plik, w którym wykonują LogFile::AddLog? Bardziej mnie by interesowała linia i plik, w którym była próba wykonania zapytania.
Pomijam już fakt, że brak wyników z zapytania nie jest żadnym błędem czy też informacją wartą logowania. To normalne, że zapytanie może nic nie zwrócić.

[Adi32]

Uczę się z eduweb php oop.

Uczyłem się swego czasu z materiałów EduWeb i prawdę mówiąc mogę polecić jestnie CSS dla osób bardzo początkujących.

[Fifi209]

ManagerDB to najczęściej singleton, wtedy przekazywanie przez konstruktor odpada.

A jaki problem z utworzeniem klasy statycznej np. Config, która z pliku wczyta do zmiennych konfiguracje DB ?
Poza tym, piszesz singleton - i znów strzał w stopę, bo nie wykonasz połączeń dla dwóch baz.

Jednak lepiej byłoby przekazywać te stałe globalną typu $config['db_default']['host']...

Wypluj to i nie namawiaj do czegoś takiego.

[!*!]

Myślałem ze ten kod będzie dobry, bo w końcu był nagrywany przez profesjonalistów (chyba?)

No tak, nie do końca jeszcze widzę sens stosowania metod statycznych.

Chciałem na tym kodzie się uczyć, mieć taką podstawę.

Profesjonaliści nie zrobiliby takiego syfu w kodzie. Im częściej widzę przykłady z ich kursów, tym bardziej mam wrażenie że piszą je początkujący. Zapoznaj się z podstawami np. PDO http://pl.wikibooks.org/wiki/PHP

[Fifi209]

!*! - wybacz ale powtarzasz to co już napisane, o PDO ja napisałem i dałem linka. O tym, że profesjonaliści tak nie robią pisał nospor