4
\$\begingroup\$

Is there any way I could improve this script, such as by writing it in fewer lines (more efficient)? I'm very interested in keeping my code dry and fast! Also, if there is a way to make it more dynamic don't hesitate to tell me!

  <?php
    /* Connect to an MYSQL database Using credentials and PDO(PHP DATA OBJECT) */
    $dsn = 'mysql:host=**;dbname=**';
    $user = '**';
    $password = '**';
    $connection = new PDO($dsn, $user, $password);
    $table = $_POST['table'];
    $querySQL = "";
    $datum = $_POST['datum'];

    //SELECT content,author FROM quote LIMIT 10
    //INSERT INTO birthday (date,name) VALUES ('2012-05-05','henk')

    switch ($table) {
        case "quote":
            $querySQL = "SELECT content,author FROM quote WHERE datum = ? LIMIT 10";
            $stmt = $connection->prepare($querySQL);
            $stmt->execute(array($datum));
            break;
        case "birthday":
            $querySQL = "SELECT date,name FROM birthday LIMIT 10 WHERE datum = ? LIMIT 10";
            $stmt = $connection->prepare($querySQL);
            $stmt->execute(array($datum));
            break;
        case "facts":
            $querySQL = "SELECT content from facts WHERE datum = ? AND category = ? LIMIT 10";
            $categorie = $_POST['categorie'];
            $stmt = $connection->prepare($querySQL);
            $stmt->execute(array($datum, $categorie));
            break;
        case "lifehacks":
            $querySQL = "SELECT content from lifehacks WHERE datum = ? LIMIT 10";
            $stmt = $connection->prepare($querySQL);
            $stmt->execute(array($datum));
            break;
    }
    if(isset($stmt)) {
        $results = $stmt->fetchAll(PDO::FETCH_ASSOC);
        $json = json_encode($results);
        echo $json;
    }
    ?>
\$\endgroup\$
1
\$\begingroup\$
$querySQL = "SELECT ";
$parameters = [$datum];
switch ($table) {
    case "quote":
        $querySQL .= "content, author FROM quote WHERE datum = ?";
        break;
    case "birthday":
        $querySQL .= "date, name FROM birthday WHERE datum = ?";
        break;
    case "facts":
        $querySQL .= "content FROM facts WHERE datum = ? AND category = ?";
        $parameters[] = $_POST['categorie'];
        break;
    case "lifehacks":
        $querySQL .= "content FROM lifehacks WHERE datum = ?";
        break;
}
$querySQL .= " LIMIT 10";
$stmt = $connection->prepare($querySQL);
$stmt->execute($parameter);

Perhaps you're looking for something such as this. You had lots of repetition in your code, and so I basically applied the DRY rules to your code. It could still use a lot of work and I'm sure there's other approaches, but this might help you kickstart the process!

Another way to possibly help improve your code would be to separate the logic, database access, and your echos. You've probably heard of frameworks and such, and you might want to start looking into your options. It will keep your code separated, and while maybe not shorter, it will be much neater!

\$\endgroup\$
1
\$\begingroup\$

This answer is Based on previous answer by Alex L.

Since $table is based on user input, you should add a "default" case to the switch :

$querySQL = "SELECT ";
$parameters = [$datum];
switch ($table) {
    case "quote":
        $querySQL .= "content, author FROM quote WHERE datum = ?";
        break;
    case "birthday":
        $querySQL .= "date, name FROM birthday WHERE datum = ?";
        break;
    case "facts":
        $querySQL .= "content FROM facts WHERE datum = ? AND category = ?";
        $parameters[] = $_POST['categorie'];
        break;
    case "lifehacks":
        $querySQL .= "content FROM lifehacks WHERE datum = ?";
        break;
    default:
        //Throw some exception or manage the error
        break;
}
$querySQL .= " LIMIT 10";
$stmt = $connection->prepare($querySQL);
$stmt->execute($parameter);

Otherwise, you will send invalid SQL to your db adapter each time $table does not contain the values you manage

\$\endgroup\$
1
\$\begingroup\$

It is odd that you are expecting an HTTP POST for a request to retrieve existing information. Normally, such requests should be HTTP GETs. At the least, you should use $_REQUEST rather than $_POST so that GET will work.

\$\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.