Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use PDO prepared statement - avoid straw man #6665

Closed
wants to merge 1 commit into from

Conversation

dr-matt-smith
Copy link
Contributor

I suggest demonstrating decent PDO code, through the use of a prepared statement
rather than setting up a 'straw man' of concatenating _GET['id'] for form an SQL string

there are very strong arguments for using Doctrine and Symfony etc. - no need to write bad PDO code to artificially strengthen the MVC argument

.. matt ..

I suggest demonstrating decent PDO code, through the use of a prepared statement
rather than setting up a 'straw man' of concatenating _GET['id'] for form an SQL string

there are very strong arguments for using Doctrine and Symfony etc. - no need to write bad PDO code to artificially strengthen the MVC argument

.. matt ..
$row = $result->fetch(PDO::FETCH_ASSOC);
$query = 'SELECT created_at, title, body FROM post WHERE id=:id';
$statement = $pdo->prepare($query);
$statement->bindParam(':id', $id, PDO::PARAM_INT);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bindValue()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you’re right,
bindValue(...) makes more sense, since statement is not being re-used with different values in a bound param-variable

regards,

.. matt ..

On 20 Jun 2016, at 13:21, Christian Flothmann notifications@github.com wrote:

In book/from_flat_php_to_symfony2.rst #6665 (comment):

@@ -254,9 +254,11 @@ an individual blog result based on a given id::
function get_post_by_id($id)
{
$link = open_database_connection();

  •    $id = intval($id);
    
  •    $result = $link->query('SELECT created_at, title, body FROM post WHERE id = '.$id);
    
  •    $row = $result->fetch(PDO::FETCH_ASSOC);
    
  •    $query = 'SELECT created_at, title, body FROM post WHERE  id=:id';
    
  •    $statement = $pdo->prepare($query);
    
  •    $statement->bindParam(':id', $id, PDO::PARAM_INT);
    
    bindValue()?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/symfony/symfony-docs/pull/6665/files/065dd3a3d49853eca691fb98003fb086a85b81c6#r67680106, or mute the thread https://github.com/notifications/unsubscribe/ABKRV9ecC9edXcKfJu2m8hGGpHyOEjl0ks5qNoXPgaJpZM4I5n_r.

Dr. Matt Smith | Senior Lecturer in Computing | matt.smith@itb.ie
Department of Informatics, School of Informatics & Engineering,
Institute of Technology Blanchardstown, Dublin 15, Republic of Ireland
www.itb.ie | Tel: +353-(1)-885-1098 | Fax: +353-(1)-885-1001

Recent publications:

  • Unity 5.x Cookbook (2015)

@javiereguiluz
Copy link
Member

👍

$result = $link->query('SELECT created_at, title, body FROM post WHERE id = '.$id);
$row = $result->fetch(PDO::FETCH_ASSOC);
$query = 'SELECT created_at, title, body FROM post WHERE id=:id';
$statement = $pdo->prepare($query);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$pdo must be $link here (or you need to rename the variable above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry my mistake - I always use $pdo
yes, $link fits with existing code

regards,

.. matt ..

On Jun 21, 2016, at 07:57 AM, Christian Flothmann notifications@github.com wrote:

In book/from_flat_php_to_symfony2.rst:

@@ -254,9 +254,11 @@ an individual blog result based on a given id::
function get_post_by_id($id)
{
$link = open_database_connection();

  •    $id = intval($id);
    
  •    $result = $link->query('SELECT created_at, title, body FROM post WHERE id = '.$id);
    
  •    $row = $result->fetch(PDO::FETCH_ASSOC);
    
  •    $query = 'SELECT created_at, title, body FROM post WHERE  id=:id';
    
  •    $statement = $pdo->prepare($query);
    
    $pdo must be $link here (or you need to rename the variable above).

    You are receiving this because you authored the thread.
    Reply to this email directly, view it on GitHub, or mute the thread.

wouterj added a commit that referenced this pull request Jul 8, 2016
This PR was submitted for the 3.1 branch but it was merged into the 2.7 branch instead (closes #6665).

Discussion
----------

use PDO prepared statement - avoid straw man

I suggest demonstrating decent PDO code, through the use of a prepared statement
rather than setting up a 'straw man' of concatenating _GET['id'] for form an SQL string

there are very strong arguments for using Doctrine and Symfony etc. - no need to write bad PDO code to artificially strengthen the MVC argument

.. matt ..

Commits
-------

7087a25 use PDO prepared statement - avoid straw man
wouterj added a commit that referenced this pull request Jul 8, 2016
@wouterj
Copy link
Member

wouterj commented Jul 8, 2016

Thanks @dr-matt-smith! I've merged this PR into the 2.7 version of the docs and I've fixed the comments in 2efd485.

As I think you may not know: If you open a PR, you open a PR from a specific branch (patch-2 in your fork in this case). If you push a new commit to that branch, the PR will be automatically updated. This would allow you to fix the comments in your PRs yourself.

@wouterj wouterj closed this Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants