3
\$\begingroup\$

I'm currently learning MySQL about prepared statements and I was wondering if this code I've written is the most efficient way.

The process:

  1. There's a query that checks a table called 'websites' for a field called websites_pagecount.

    The same query then increments the field by 1. (I haven't done this is the database using auto-increment because there will be more than one row with the same number.)

  2. There's a query that then runs and grabs the new number.

  3. Then another query runs and creates a row in a table called pages and sets the new pagecount number in to the page_id field (again, I haven't used auto-increment on this as there will be more than one row with the same number).

However, it all seems a bit 'cumbersome'.

$mysql['stmt'] = $mysql['conn']->prepare("UPDATE websites SET website_pagecount=website_pagecount+? WHERE website_domain=?");
$mysql['stmt']->bind_param("is", $a = 1, $website['website_id']);
$result = $mysql['stmt']->execute();
$mysql['stmt']->close();


$mysql['stmt'] = $mysql['conn']->prepare("SELECT website_pagecount FROM websites WHERE website_domain = ?");
$mysql['stmt']->bind_param("s", $website['website_id']);
$result = $mysql['stmt']->execute();
//GET ROWS
$result = $mysql['stmt']->get_result();
$pagecount = $result->fetch_assoc();
$mysql['stmt']->close();


$mysql['stmt'] = $mysql['conn']->prepare("INSERT INTO pages (page_id,page_parent, page_title, page_description,page_keywords, website_id, page_layout) VALUES (?,?,?,?,?,?,?)");
$mysql['stmt']->bind_param("issssss",$pagecount['website_pagecount'],$item['parentpage'],$item['pagetitle'],$item['pagedescription'],$item['pagekeywords'],$website['website_id'],$item['pagelayout'] );
$result = $mysql['stmt']->execute();
$mysql['stmt']->close();

The code is used to create a new web page from a web based administration panel. The admin panel can be used to edit multiple websites (depending on which user is logged in) All website information is held in the 'websites' table and all the pages from all the websites are held in the 'pages' table.

Every page has an auto-incrementing unique ID, however they also have a page id relative to the website they belong to. IE (test.com can have pages with ids 1,2 and 3 however testagain.com can also have pages with ids 1,2,3)

The reason for the 'website_pagecount' field is so when a new page is created for a particular website, it counts up in order.

The reason I have done this is because if i used a 'row count' function, for example, to generate the page ID field, and pages with the ID's 1 & 3 got deleted, when a new page was generated it would set the ID to '2' - however that would conflict with the already existing page that has an ID set which was 2.

If there was to be 30-40 pages for a website and pages got deleted and created over time, there would be an awful lot of conflicts eventually per website.

\$\endgroup\$
5
  • \$\begingroup\$ $mysql['stmt'] = - do you refer to this object after executing it? Showing your whole function rather than a these few lines would be helpful. bind_param("is", $a = 1 - why bind a constant at all? \$\endgroup\$
    – AD7six
    Nov 5, 2015 at 8:51
  • \$\begingroup\$ "$mysql['stmt'] = - do you refer to this object after executing it?" - could you elaborate on this question? the query executes as required & writes the information required in to the database. ------ "bind_param("is", $a = 1 - why bind a constant at all?" - I originally had 'bind_param("is",1' however this failed, is there a way i can insert this integar without binding it at all? - to my understanding data had to be binded when using a prepared statement? \$\endgroup\$
    – PaulF
    Nov 5, 2015 at 9:23
  • 1
    \$\begingroup\$ The question is why are you using an array to store the statement object, which you repeatedly redefine/overwrite (and then probably don't use or refer to thereafter)? That's rather nonsensical. I'm asking why you're not doing prepare("UPDATE websites SET website_pagecount=website_pagecount+1 - there's no need to make something fixed a variable - unless it is in fact variable, variables in a prepared statement are arbitrary - there's no rule that says all set x=? have to be bound parameters. \$\endgroup\$
    – AD7six
    Nov 5, 2015 at 11:14
  • \$\begingroup\$ Ahh i see, sorry. When i tried 'prepare("UPDATE websites SET website_pagecount=website_pagecount+1' before it came up with an error however i just tried it again and its now working - i must of done something wrong, it was a late night :p \$\endgroup\$
    – PaulF
    Nov 5, 2015 at 23:35
  • \$\begingroup\$ re using '$mysql['stmt']' - i was learning prepared statements from w3schools and there sample code was ' $stmt = $conn->prepare("INSERT INTO ...' however I had '$mysql['']' already setup to hold the username, pass, db etc etc and as the variable for the statement was relating to mysql, i wanted to encapsulate this in to '$mysql['']' rather than just using '$stmt' - I just find it easier to read personally - does doing this have a negative effect on efficiency? \$\endgroup\$
    – PaulF
    Nov 5, 2015 at 23:35

1 Answer 1

1
\$\begingroup\$

Thanks for the comments. I understand what your webpage does now.

Suggestion #1 - Descriptions Of What Your Code Does - You might want to create some functions (or add some comments) just to make it more clear what each segment of code does. The OOP people like to make tons of functions that describe chunks of code, and the non-OOP people like comments. You can go either way, in my opinion. In my example here, I will use functions.

Suggestion #2 - More Whitespace - I would also add some spaces after your commas and maybe some enters after your commas in your bind_param() functions so that everything wraps nicely and is easy to read. You can do the same thing with your prepare() functions. It isn't a big deal for single line SQL statements like "SELECT * FROM table WHERE field = 'value'", but it starts becoming very helpful when you're doing long, complex INNER JOINS and other complicated SQL statements.

<?php

// control panel for multiple websies

$website = define it up here;
$item = define it up here;

function add_page($website_id, $item, $mysql)
{
    $next_page_count = increment_and_return_next_page_count($website_id, $mysql);

    create_new_page($item, $next_page_count, $mysql);
}

function increment_and_return_next_page_count($website_id, $mysql)
{
    $mysql['stmt'] = $mysql['conn']->prepare("
        SELECT website_pagecount
        FROM websites
        WHERE website_domain = ?
    ");
    $mysql['stmt']->bind_param(
        "s",
        $website_id
    );
    $result = $mysql['stmt']->execute();
    $result = $mysql['stmt']->get_result();
    $pagecount = $result->fetch_assoc();
    $mysql['stmt']->close();

    $pagecount++;

    $mysql['stmt'] = $mysql['conn']->prepare("
        UPDATE websites
        SET website_pagecount=?
        WHERE website_domain=?
    ");
    $mysql['stmt']->bind_param(
        "is",
        $pagecount,
        $website_id
    );
    $result = $mysql['stmt']->execute();
    $mysql['stmt']->close();

    return($pagecount);
}

function create_new_page($item, $next_page_count, $mysql)
{
    $mysql['stmt'] = $mysql['conn']->prepare("
        INSERT INTO pages (page_id,page_parent, page_title, page_description,page_keywords, website_id, page_layout)
        VALUES (?,?,?,?,?,?,?)
    ");
    $mysql['stmt']->bind_param(
        "issssss",
        $next_page_count,
        $item['parentpage'],
        $item['pagetitle'],
        $item['pagedescription'],
        $item['pagekeywords'],
        $website['website_id'],
        $item['pagelayout']
    );
    $result = $mysql['stmt']->execute();
    $mysql['stmt']->close();
}

add_page($website['website_id'], $item, $mysql);

?>

Other than those tips, I will defer to more experienced PHP coders. I hope my ideas help you out a little bit though. Good luck!

P.S. As for your original idea of combining the statements: I don't think they can be combined because they are doing different things. One is SELECT, another is UPDATE, and the third is INSERT. If they were all doing the same thing, then you could combine them using various types of JOINS probably. But since they are performing different actions I don't think that is an option.

\$\endgroup\$
6
  • \$\begingroup\$ The code is used to create a new web page from a web based administration panel. The admin panel can be used to edit multiple websites (depending on which user is logged in) All website information is held in the 'websites' table and all the pages from all the websites are held in the 'pages' table. Every page has an auto-incrementing unique ID, however they also have a page id relative to the website they belong to. IE (test.com can have pages with ids 1,2 and 3 however testagain.com can also have pages with ids 1,2,3) \$\endgroup\$
    – PaulF
    Nov 5, 2015 at 9:38
  • \$\begingroup\$ Ah. I get it. This is code to add a page to a website from a control panel with multiple websites. If I get time I'll edit my answer to reflect this. And if I get even more time than that I'll do it in PDO for you ; ) \$\endgroup\$ Nov 5, 2015 at 9:41
  • \$\begingroup\$ The reason for the 'website_pagecount' field is so when a new page is created for a particular website, it counts up in order. The reason ive done this is because if i used a 'row count' function, for example, to generate the page ID field, and pages with the ID's 1 & 3 got deleted, when a new page was generated it would set the ID to '2' - however that would conflict with the already existing page that has an ID set which was 2. If there was to be 30-40 pages for a website and pages got deleted and created over time, there would be an awful lot of conflicts eventually per website. \$\endgroup\$
    – PaulF
    Nov 5, 2015 at 9:43
  • \$\begingroup\$ Thank you for your reply though, ill have a look through and see if i can apply the logic to this and see what i come out with ; ) \$\endgroup\$
    – PaulF
    Nov 5, 2015 at 9:45
  • \$\begingroup\$ @PaulF Thanks for the explanation. I've updated my answer to be more helpful. \$\endgroup\$ Nov 5, 2015 at 10:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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