Sorry, but this is bugging me and I have to ask. I can't figure out why this code is in this order...
PHP Code:
$pid = intval($mybb->input['pid']);
$query = $db->query("SELECT * FROM ".TABLE_PREFIX."posts WHERE pid='$pid'");
$post = $db->fetch_array($query);
$tid = $post['tid'];
$query = $db->query("SELECT * FROM ".TABLE_PREFIX."threads WHERE tid='$tid'");
$thread = $db->fetch_array($query);
$thread['subject'] = htmlspecialchars_uni($thread['subject']);
$fid = $thread['fid'];
makeforumnav($fid);
addnav($thread['subject'], "showthread.php?tid=$thread[tid]");
addnav($lang->nav_editpost);
$forumpermissions = forum_permissions($fid);
$query = $db->query("SELECT * FROM ".TABLE_PREFIX."forums WHERE fid='$fid'");
$forum = $db->fetch_array($query);
if(!$post['pid'])
{
����error($lang->error_invalidpost);
}
Eleven full lines of code assuming that $pid was valid (and that the query returned something), and THEN you get around to checking it? WTF?
![[Image: xso_undecided.gif]](https://camo.mybb.com/6936ecf4fab631869c787ad3628710a245dc1b1f/687474703a2f2f6d736768656c702e6e65742f696d616765732f736d696c6965732f78736f5f756e646563696465642e676966)
(That example is from editpost, but a similar thing is done in newthread and newreply)
Well, the eror checking could be performed earlier. I guess that is just how it was written when it first was.
I'll change that - it'll save some querying for invalid posts.
Chris Boulton Wrote:I guess that is just how it was written when it first was.
Hmm... yeah, you're right, I just checked the original RC2 code and the same problem existed, but there just weren't as many lines separating the query and the check. =/
Quote:I'll change that - it'll save some querying for invalid posts.
Cool.
![[Image: mininana.gif]](https://camo.mybb.com/32973faa604a494afdc0bf9ab639b71d1c616ec8/687474703a2f2f6d736768656c702e6e65742f696d616765732f736d696c6965732f6d696e696e616e612e676966)
Check out newreply and newthread as well... although the problem isn't as bad there, I still think it makes more sense to check the result of the query ASAP.
Now, while I'm questioning your code (
![[Image: refuck.gif]](https://camo.mybb.com/2a7dd68aba4482239220aeea3b5d63929f7735dd/687474703a2f2f6d736768656c702e6e65742f696d616765732f736d696c6965732f72656675636b2e676966)
), what's up with the "give each bracket its own line" style of coding? For example...
Copied from MyBB 1.0
PHP Code:
if($onlinecount != 1)
{
����$onlinebit = $lang->online_online_plural;
}
else
{
����$onlinebit = $lang->online_online_singular;
}
if($membercount != 1)
{
����$memberbit = $lang->online_member_plural;
}
else
{
����$memberbit = $lang->online_member_singular;
}
if($anoncount != 1)
{
����$anonbit = $lang->online_anon_plural;
}
else
{
����$anonbit = $lang->online_anon_singular;
}
if($guestcount != 1)
{
����$guestbit = $lang->online_guest_plural;
}
else
{
����$guestbit = $lang->online_guest_singular;
}
PHP Code:
if($onlinecount != 1) {
����$onlinebit = $lang->online_online_plural;
} else {
����$onlinebit = $lang->online_online_singular;
}
if($membercount != 1) {
����$memberbit = $lang->online_member_plural;
} else {
����$memberbit = $lang->online_member_singular;
}
if($anoncount != 1) {
����$anonbit = $lang->online_anon_plural;
} else {
����$anonbit = $lang->online_anon_singular;
}
if($guestcount != 1) {
����$guestbit = $lang->online_guest_plural;
} else {
����$guestbit = $lang->online_guest_singular;
}
![[Image: blah!.gif]](https://camo.mybb.com/8b3a93861ff6ad5b31946920cafe412968e70478/687474703a2f2f6d736768656c702e6e65742f696d616765732f736d696c6965732f626c6168212e676966)
PHP Code:
$onlinebit = ($onlinecount != 1) ? $lang->online_online_plural : $lang->online_online_singular;
$memberbit = ($membercount != 1) ? $lang->online_member_plural : $lang->online_member_singular;
$anonbit�� = ($anoncount != 1)�� ? $lang->online_anon_plural�� : $lang->online_anon_singular;
$guestbit��= ($guestcount != 1)��? $lang->online_guest_plural��: $lang->online_guest_singular;
Which is easier to read?
Personally I hate the excessively-spaced-out code... it's harder to follow because you can't see as much of it on your screen at once and you have to scroll more. I'm just curious if there's some kind of reasoning behind the decision to use that style.
Since we are on the topic of optimising code. How about a reply to
this?