I was just looking at the print_page() function and just saw something I thought was wrong. Haven't bothered loading it up to check it yet, but the "and" should be an "or" shouldn't it?

It probably won't make a difference, because most of the time if one fails the other will to - but why use "and" when either problem would cause the script to stop working?


should be

Cheers,
Michael Bray

Surely you only need:

...because it will trigger the error if the the file or directory doesn't exist.

Yeah, that's right.
I'll fix those up.


Adrian

Actually going through it again, making the code not warn with E_ALL (like a use strict w/ Perl) turned on, it turns out you have to do a file_exists() before doing a is_file/dir/etc.


Adrian

But if you do the file_exists without using the is_dir it won't produce an error with E_ALL. file_exists should be enough checking...
Cheers,
Michael Bray

Not really. file_exists() tells you if there's a file or directory or even a symlink exists. You should check to see what exactly it is. No use trying to parse a directory


Adrian

I know what you are saying mate,

But when you consider that the code is:

if ((file_exists("$admin_root_path/templates/${template_set}_php/$_file.html")

which leads the to the template directory, and has the .html extension on it - it is surely going to be a file, unless someone sets out to try to trick it. Each function used is going to slow it down, so you want print_page to work well, but at the same time be very fast.
Cheers,
Michael Bray

I doubt that extra call would be much of a performance hit (or any at all) since the result of file_exists() is cached.


Adrian

Yep, the extra time it takes is negligible. Here's the results of a quick test I did:

10000 iterations of is_readable:
(0.056605935096741s)
(0.056142091751099s)
(0.05635404586792s)
(0.056273937225342s)
(0.056421995162964s)
(0.10010099411011s)
(0.11135601997375s)
(0.091256976127625s)
(0.064092993736267s)
(0.10032200813293s)
average: 0.074892699718475s

10000 iterations of is_readable and is_dir:
(0.088375926017761s)
(0.094772934913635s)
(0.085157036781311s)
(0.093469023704529s)
(0.09726893901825s)
(0.088521957397461s)
(0.095229029655457s)
(0.085625052452087s)
(0.080011963844299s)
(0.094208002090454s)
average: 0.090263986587524s

That's only 0.015 seconds difference on 10000 iterations. Very little difference when you consider the number of calls to print_page there are for each page generated.
Never mind my blabbing, I was just bored...


Adrian

If you run a small ammount of code though a loop a lot of times it is going to get cached which will through off the benchmarks a bit anyway - but you'll see that it is about 15% faster when you don't use is_dir. When you consider the actual ammount of code used throughout Links each little thing does make a difference.

The main point is that it seems to be that it is placed there for the sake of being there. Since the code already has the .html extension built in, if the file_exists turns out alright - then it is going to work.

Anyway. It doesn't really matter - its just that I like functions that are used on basically every page to be as optimised. Its only a small difference in time as you pointed out - but yeah I'd leave it out.


Cheers,
Michael Bray