0
\$\begingroup\$

I made a function that contains the database connection and query. Now I'm always calling this function every time I need to query a database. Is it good or is there another way better?

Database connection:

    /* Database Configuration */
DEFINE( "DATABASE_HOST", "" );
DEFINE( "DATABASE_USERNAME", "" );
DEFINE( "DATABASE_PASSWORD", "" );
DEFINE( "DATABASE_ESSENTIALS", "" );

        /* Database Connection & Data Manipulation */
    function connect_database( $database_id ) {
        try {
            $databases = array( "essentials"  => DATABASE_ESSENTIALS );

            $connection = new PDO( "mysql:host=" . DATABASE_HOST . ";dbname=" . $databases[ $database_id ], DATABASE_USERNAME, DATABASE_PASSWORD );
            $connection->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
        } catch ( PDOException $exception ) {
            trigger_error( $exception->getMessage() );
        }

        return $connection;
    }

    function prepare_and_execute_query( $database_id, $query, $statement_id, $values ) {
        $statement = $this->connect_database( $database_id )->prepare( $query );

        if ( $statement_id == "get_system_configuration" ) {
            $statement->bindParam( ":system_configuration_id", $values[ 0 ] );
        }

        $statement->execute();

        return $statement;
    }

    function get_system_configuration( $system_configuration_id ) {
        return ( ( $system_configuration_row = $this->prepare_and_execute_query( "essentials", "SELECT value FROM `[main]system_configurations` WHERE system_configuration_id = :system_configuration_id", "get_system_configuration", array( $system_configuration_id ) )->fetch() ) ? ( ( !( empty( $system_configuration_row[ "value" ] ) ) ) ? ( $system_configuration_row[ "value" ] ) : ( $system_configuration_row[ "default_value" ] ) ) : ( trigger_error( "Undefined system configuration id" , E_USER_ERROR ) ) );
    }

Sample usage of system configuration function:

<!DOCTYPE html>
<html lang="<?php echo $essentials_global->get_system_configuration( "html_lang" ); ?>">
| improve this question | |
\$\endgroup\$
1
\$\begingroup\$

A couple simple notes:

  • your code is very hard to read because the lines are so long. Strive to keep each line under ~80 characters.
  • most of the time in more complex applications, DB connection management is put into a class or classes, not functions. This better allows you to do things like share a single DB connection in different parts of the app, rather than instantiating a new connection every time you need to query the database. That is an expensive operation.
  • Your method that executes the prepared statement considers only happy path. What if statement preparation or execution fails? Also, it seems strange this this function then "leaks" the underlying PDO implementation to the method caller, requiring the caller to deal with getting the query result set from the PDOStatement object.
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.