Nuke PHP Coding Conventions: Questions and Observations

Posted: Tue Oct 03, 2006 11:28 am

I've been hacking on my site for a few months now. I am new to PHP and MySQL. I also understand Nuke goes back a long time and PHP has evolved several times. But I find curious things in the code. For example, in the Top module (and other places), great care is taken to ensure that $result variables from $db->sql_query() calls are not reused. For example, you see $result1, then $result2, then $result3 when no attempt is made to resuse $result1 ever again in the file. If it were me, I would just reuse $result = $db->sql_query() every time. What are they trying to avoid by overwriting $result? Is there some garbage collection that happens if it is reused that they want to delay until after the page has been loaded? Is that a significant cost? I haven't measured it, but I haven't seen any slow down in my own code when I reuse $result.

Also, I see a lot of this:

Code:




$sql = "select * from " . $prefix . "_authors where ...";

Maybe this is just personal taste, but I find it easier to type:

Code:




$sql = "select * from {$prefix}_authors where ...";

Is there something about this style that is "bad" or was it just not in FB's tool bag?

And finally, when writing my own modules, I have found PHP's "Here document" syntax extremely helpful when echo'ing out lots of HTML. I don't have to back-quote quotation marks and it still does variable expansion. E.g.

Code:




$echo <<<END_HTML


<!-- I tried to put HTML here but got the nasty Sentinel black page;


 But hopefully you know what I am talking about... -->


END_HTML;

I don't see any of that in the Nuke code base. Is that a new'ish feature to PHP or FB just didn't know about it? It sure makes things easier to read and I really like not having to escape " characters. You can actually format the HTML nicely too.

Posted: Tue Oct 03, 2006 1:48 pm

I didn't know about the heredoc syntax till you brought it up. The PHP manual says it was added in PHP 4, so maybe that's why you don't see it in PHPnuke. I'm not sure but I'd bet that PHPnuke was started under PHP3. The use of single quotes versus double quotes in "escaping" variables is one of the trickier aspects of PHP when you are getting started but you get used to it. For me at least "getting used to it" means I pretty quickly say "oh, I screwed up again". It all depends on how much coding I'm doing in a given time period, if it's a lot I get efficient and if it's not I get rusty.

I also didn't know that braces were an alternative to double quotes. But reading the manual it appears they are. I don't think it makes much difference which you use and you are right that the braces could look "cleaner" but I suspect that a lot of people just copy and paste some existing code so whatever FB started by using winds up getting replicated over and over.

As to $result and $result2 ... 3 etc. Yeah: if you aren't going to reuse the results from $result then you might as well reuse $result. Smile

On the other hand having $result2 and 3 and 4 etc. is probably not a very big inefficiency in the overall scheme of things. Again, I suspect that a lot of times in the development of some of these programs FB didn't know in advance what he was doing (probably still doesn't) so he probably figured "just use another version of $result and be safe". I've done it both ways in my programs.

Those are good questions and thanks to pointing me to some things that I didn't know existed.

edit: oh by the way, if you expect to see any "standards" for Nuke programming you will have to look long and hard. Just look at the way themes communicate the side that a block should be on with mainfile. Sometimes it's left or it could be LEFT or it could be "left" or perhaps l (which is hard to distinguish from the number 1) and the same goes for center, down, and right. Because of that mess mainfile has to do a strtolower and then pick the first letter out of the resulting string array and use that. Trying to get it changed is a real chore because even if you change mainfile, you need to change every theme that communicates with it and who knows when someone is going to bring in a new theme that doesn't work right and "wahhh wahhh wahhh -- my theme doesn't work." I'm going to keep working on Raven on that change though. Maybe by RN 3 we'll have it.

Posted: Tue Oct 03, 2006 9:30 pm

Most of those were from the Patched files actually, in order to differentiate one set of queries from the next. So for moding, it is easier to differentiate than the standard

Code:




$result = $db->sql_query($sql);

line that would be used multiple times

I didn't know about the brackets, but generally variable insertion into a string is more efficient by concatentation. Concatenation is generally easier, PHP does not have to process the entire string and pull out the variables for replacement. It knows exactly what is connected to what through concatenations.

Posted: Wed Oct 04, 2006 11:28 am

evaders99 wrote:

I didn't know about the brackets, but generally variable insertion into a string is more efficient by concatentation. Concatenation is generally easier, PHP does not have to process the entire string and pull out the variables for replacement. It knows exactly what is connected to what through concatenations.

True but very often there are variables already being substituted and double quotes are used when not needed, e.g. a typical line from some Nuke code

Code:




$sql = "SELECT * from " . $prefix . "_something WHERE field1 = $id";

If they were concerned about efficiency they would have written it like:

Code:




$sql = 'SELECT * from ' .  "$prefix" . '_something WHERE field1 = ' .  "$id";

(note the use of single quotes instead of double quotes)

If you use double quotes PHP has to scan the string for variable substitutions even if you don't have any! Single quotes means do not do variable substitution. I very frequently see the use of double quotes when technically only single quotes could have been used.

In any event, I don't think for most things you should worry about the cost of variable substitution. Those costs are very small compared to the database access and other things going on in a typical page of php. (But of course they may add up). Trying to always use single quotes when you can is a big headache and makes maintenance difficult too.

And yes, that "here document" syntax is very nice!

Posted: Wed Oct 04, 2006 1:13 pm

True single quote is something we're trying to go to as well. It may not add much overhead, but any small increase will help.

Posted: Wed Oct 04, 2006 2:29 pm

Actually, the most efficient and proper way to code this:

Quote:

If they were concerned about efficiency they would have written it like:

Code: ‹ Select ›

$sql = 'SELECT * from ' . "$prefix" . '_something WHERE field1 = ' . "$id";

(note the use of single quotes instead of double quotes)

Would really be:

Code:




$sql = 'SELECT * from '.$prefix.'_something WHERE field1=\''.$id.'\'';

This is pure concatenation. The use of double quotes around a variable is unnecessary and actually forces PHP to still parse the string rather than just substituting the value in the concatenation. We have started already to do this alot within RavenNuke 2.10 coming out soon (not wide-spread, but as we touch scripts).

Posted: Wed Oct 04, 2006 2:31 pm

BTW, we have replaced double-quotes to single quotes on almost ALL the language defines and you would be absolutely amazed at how much quicker RavenNuke 2.10 is from 2.02.02!

Posted: Wed Oct 04, 2006 2:47 pm

Ah...I see what you mean montego. Thanks.

You really see a difference converting over to single quotes? Wow, thanks for the data point. I will start doing that too.

registered · Posted: Wed Oct 04, 2006 5:46 pm

Here's another: language constants vs array Laughing

If you know your PHP you will notice that defining array data is faster then defining constants.

Posted: Wed Oct 04, 2006 8:10 pm

djmaze wrote:

Here's another: language constants vs array Laughing

If you know your PHP you will notice that defining array data is faster then defining constants.

Can you give an example please?

Posted: Wed Oct 04, 2006 9:25 pm

I would have to see an example as well. Wouldn't a constant need less processing because it doesn't change?

Posted: Thu Oct 05, 2006 9:33 am

montego wrote:

BTW, we have replaced double-quotes to single quotes on almost ALL the language defines and you would be absolutely amazed at how much quicker RavenNuke 2.10 is from 2.02.02!

" = The parser is going to check for variables in the code, thus it takes longer
' = The parser is not going to check for variables, thus it takes less time

Posted: Thu Oct 05, 2006 4:13 pm

LOL, you keep sending me back to the text books Montego. Where you quoted:

Code:

$sql = 'SELECT * from '.$prefix.'_something WHERE field1=\''.$id.'\'';

I was staring at it and staring and staring and then I went in and looked at some examples in the newest mainfile. The problem is that if you look at two single quotes next to each other on the screen, at least with my aging eyes, they look pretty identical to one double quote. So I was looking at the '''.$id.'\: and going what the heck? but then I realized it is really: escape single quote single quote period $id period single quote escape single quote single quote with the last single quote ending the SELECT statement. And that is perfectly logical.

Thanks Gremmie for starting this thread, it's been very instructive, at least to me.

Posted: Thu Oct 05, 2006 4:26 pm

Yeah, I learned two new things. So did we decide that

Code:

{$prefix}

Is just easier to type?

Posted: Fri Oct 06, 2006 2:52 pm

Why does anyone need an example? Can't you write it yourself? It is so d*** easy....

Code:




<?php # needs php5


$time = microtime(true);


for ($i = 0; $i < 10000; ++$i)


{


   define("TESTSTRING$i", "this has a value of $i");


}


echo 'define: '.round(microtime(true)-$time,4).' seconds<br>';





$time = microtime(true);


$array = array();


for ($i = 0; $i < 10000; ++$i)


{


   $array["TESTSTRING$i"] = "this has a value of $i";


}


echo 'array: '.round(microtime(true)-$time,4).' seconds';

output:

Code:

define: 0.0808 seconds


array: 0.0603 seconds

evaders99 wrote:

Wouldn't a constant need less processing because it doesn't change?

No, the issue lies within the permissions and scope where PHP has to define these things.

Posted: Fri Oct 06, 2006 4:46 pm

Removing " \' " seems to work fine for me

Code:

$sql = 'SELECT * from '.$prefix.'_something WHERE field1='.$id.'';

instead of:

Code:

$sql = 'SELECT * from '.$prefix.'_something WHERE field1=\''.$id.'\'';

am I missing something?

Posted: Fri Oct 06, 2006 5:35 pm

Yeah, I think you are missing something that maybe Montego or Evaders can explain better than I. You want that last $id to be "encapsulated" inside of single quotes to avoid injections from outside. So, the first single quote is escaped and the remaining '.$id' is enclosed in single quotes so that someone can't pass in a "fake" value for $id.

Posted: Fri Oct 06, 2006 5:39 pm

Aha! Now it starts to make some sence Smile

Posted: Fri Oct 06, 2006 6:28 pm

But if you enclose it in single quotes, variable substitution is not performed.

Posted: Fri Oct 06, 2006 6:41 pm

Gremmie wrote:

But if you enclose it in single quotes, variable substitution is not performed.

not sure what this relates to.... sorry, I'm relatively new to that stuff... was it something in reply to what I had posted? what "it" means then?

Posted: Fri Oct 06, 2006 6:47 pm

Well there are too many things flying around here, so here is a summary of how to efficiently build an SQL query string:

Code:




$sql = 'SELECT * from '.$prefix.'_something WHERE field1=\''.$id.'\'';

Note that the double quotes above are not really double quotes, they are the \, ', and ' in succession.

This technique uses single quotes and no double quotes, so it is somewhat faster because PHP does not have to look for variable substitutions. And inside the SQL string, the value of id will be placed inside single quotes to help minimize bad things if the variable $id was poisoned with rogue SQL.

Posted: Fri Oct 06, 2006 6:49 pm

fkelly wrote:

You want that last $id to be "encapsulated" inside of single quotes to avoid injections from outside.

Does the same logic refers to $prefix?
And, does the following string make sence, or \' should be employed here, also?

Code:

$MyNewDesc = 'List of terms contained in '.$en_title.' encyclopedia under letter '.$ltr.'';

(all quotes are single)
sorry my ignorance Smile

Posted: Fri Oct 06, 2006 7:07 pm

Misha, it is not an "implementation question" really, but a string definition question. Everywhere that you are building a string, it is much faster to concatenate rather than parse.

BTW, someone mentioned HEREDOC syntax previously. It is going to be much slower as well due to the parsing that PHP must do for variable substitution. However, everything is life is about trade-offs... in some cases, you may not care about a few miliseconds or even a second (such as an admin screen), while in other cases you might prefer easier coding and readbility.

Your choice to make...

Posted: Fri Oct 06, 2006 7:22 pm

djmaze, take it easy friend... Wink

Now look at this:

Code:




<?php # needs php5 


$time = microtime(true); 


for ($i = 0; $i < 10000; ++$i) 


{ 


   define("TESTSTRING$i", "this has a value of $i"); 


} 


echo 'define: '.round(microtime(true)-$time,4).' seconds<br>'; 





$time = microtime(true); 


$array = array(); 


for ($i = 0; $i < 10000; ++$i) 


{ 


   $array["TESTSTRING$i"] = "this has a value of $i"; 


} 


echo 'array: '.round(microtime(true)-$time,4).' seconds<br>'; 





$time = microtime(true); 


for ($i = 0; $i < 10000; ++$i) 


{ 


   $array["TESTSTRING$i"] = "this has a value of $i"; 


} 


echo 'parsing: '.round(microtime(true)-$time,4).' seconds<br>'; 





$time = microtime(true); 


for ($i = 0; $i < 10000; ++$i) 


{ 


   $array["TESTSTRING$i"] = 'this has a value of '.$i; 


} 


echo 'concatenation: '.round(microtime(true)-$time,4).' seconds'; 





?>

The results I got were:

define: 0.1545 seconds
array: 0.1146 seconds
parsing: 0.0663 seconds
concatenation: 0.0262 seconds

This clearly demonstrates the tremendous overhead incurred with PHP having to parse the variables!

Posted: Fri Oct 06, 2006 7:46 pm

montego,

nice that you extended my example by putting parsing and concatenation in there as well.
It shows that many people still need to learn a lot about PHP Very Happy

For example, how many times did you see something like: $time = "$file"