0

I have the following chunk of code to do one of several things, the most important being to set up the main loop from which we will grab db contents (depending on conditions).

These conditions are: whether or not we are looking at a category - whether or not we are looking at an ID - whether or not we are looking at neither (the front page) - whether or not the admin password/username combination is used (for deleting/editing purposes).

$replies is grabbing (if we're looking at an ID) all entries that have a PARENT of the ID being called. $quickinfo is used for setting things like meta tags (title, keywords, etc.)

My only (current) question being: is this efficient? If not, why not.

$selection = "ID,CONTENT,IP,SUBJECT,CATEGORY,APPROVED,DATE,PARENT,
              PASSWORD,USERNAME,THANKS,DISAPPROVE,IPS,BUMPS";

$id = strip_tags($id);
$category = strip_tags($category);
$threads = mysql_query("SELECT COUNT(*) FROM $board") or die();
list($threadsTotal) = mysql_fetch_row($threads);
$threadsTotal_pages = ceil($threadsTotal / $POSTSPERPAGE);
$threadsPage = intval(@$_GET["page"]);

if (0 == $threadsPage)
{
    $threadsPage = 1;
}

$threadsStart = $POSTSPERPAGE * ($threadsPage - 1);
$threadsMax = $POSTSPERPAGE;

if ($category > "" && $id == ""
    && $passw != <ONE MISTAKE HERE> $adminPass 
    && $username != <ANOTHER MISTAKE HERE> $adminID)
{
    $threads = mysql_query("SELECT $selection FROM $board WHERE PARENT=0
                             AND CATEGORY='$category'
                             ORDER BY ID DESC LIMIT $threadsStart, $threadsMax");
}

if ($category == "" && $id == "" &&
    $passw !== <NOT EQAULS DOES NOT REQUIRE TWO EQUAL SIGNS AGAIN>
    $adminPass && $username != <AND AGAIN> $adminID)
 {
     $threads = mysql_query("SELECT $selection FROM $board WHERE PARENT=0
                              ORDER BY ID DESC LIMIT $threadsStart, $threadsMax");
 }

 // PLEASE CHECK THE NOT EQUALS FUTHER ON....

 if ($id > "" && $passw != $adminPass && $username != $adminID)
 {
     $threads = mysql_query("SELECT $selection FROM $board WHERE PARENT=0
                             AND ID=$id LIMIT 1");

     $quickinfo = mysql_query("SELECT COUNT(*) FROM $board") or die();
     $quickinfo = mysql_query("SELECT ID,CONTENT,SUBJECT,CATEGORY,USERNAME FROM
                               $board WHERE PARENT=0 AND ID=$id LIMIT 1");

     $replies = mysql_query("SELECT COUNT(*) FROM $board") or die();
     $replies = mysql_query("SELECT $selection FROM $board WHERE PARENT=$id
                             ORDER BY ID ASC");

     while (list( $ID, $CONTENT, $SUBJECT, $CATEGORY, $USERNAME) =
            mysql_fetch_row($quickinfo))
     {
         $threadID = $ID;
         $threadContent = $CONTENT;
         $threadSubject = $SUBJECT;
         $threadCategory = $CATEGORY;
         if ($USERNAME > "")
         {
             $threadAuthor = $USERNAME;
         }
         elseif ($USERNAME == "")
         {
             $threadAuthor = "Anonymous";
         }
     }

 } // That is the end of your if

 if ($passw == $adminPass && $username == $adminID)
 {
     $threads = mysql_query("SELECT $selection FROM $board ORDER 
                             BY APPROVED DESC LIMIT $threadsStart,
                                                    $threadsMax");
 }
2
  • Please could you make the code more readable by using the space bar and a few strategic enter keys. i.e. so I can read it would the use of the horizontal scroll bar. Commented Oct 12, 2012 at 22:44
  • Hopefully that's slightly better than the original was. Commented Oct 12, 2012 at 22:56

2 Answers 2

1

Matt- Reformatted code and also spotted that you use '!==' for '!-'

Also you should consider using mysqli or POD. mysql library for PHP is deprecated.

Sign up to request clarification or add additional context in comments.

6 Comments

Admittedly I can be a little absent-minded when it comes to the !== problem. (Looking at some of my other code, in some places I use != while in others !== shows up.)
Please be a bit more appreciative and give a +1 for the formatting and the spotting of a potential problem.
Sorry, it's not that I'm not appreciative, I just had dinner spring up and my priorities were re-arranged.
@Matt - I will bear that in mind in future. Also my daughters also spring up, mostly over my shirt!
So I looked into mysqli - this would require a complete rewrite of pretty much any mysql code I'm using at the moment, and as of right now (having just re-written the overall code 3x in the last 2 months, this being the third) I am not entirely ready to do that just yet. Are there any resources that you can think of that will help ease the pain of transferring any mysql code I have currently over to mysqli?
|
0

I think I see one issue. I believe you really want this

if ($category > "" && $id == ""
    && $passw != <ONE MISTAKE HERE> $adminPass 
    && $username != <ANOTHER MISTAKE HERE> $adminID)

{

to be this

if ($category > "" && $id == ""
    && ($passw != <ONE MISTAKE HERE> $adminPass 
    || $username != <ANOTHER MISTAKE HERE> $adminID))

{

or possibly, you might want another || in place of another &&, I am not completely certain of the logic there of the category and ID.

Otherwise, you only query the DB once on the page which seems like a pretty efficient setup.

2 Comments

Since I have a combination of the Admin ID AND the admin pass, I want to make sure both are present and correct. Otherwise, anyone could just use the admin ID as a log in credential and it would say "oh hey admin, what'd you want to do?" So in this case, we're checking that both of the parameters are met. Now, since I've got a loop specifically FOR the admin/password combination, I wanted to exclude the one above (otherwise it would show up twice.)
right, so this should be if either doesn't match, not if both don't, so or, not and?

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.