Linked by Eugenia Loli on Thu 24th Apr 2008 22:45 UTC
Internet & Networking We were always proud of OSNews' (and Gnomefiles.org's) mobile capabilities. We spent years collecting keywords to be able to automatically redirect or serve a mobile-formatted or WAP-formatted (wap.osnews.com) page to less capable browsers. We believe that this script can recognize 99% of the world's non-desktop browsers. We gave special care to not only phones, but PDAs, gaming devices, text browsers, even weird embedded systems browsers that most users have never heard of. Now, it's time to open source our PHP detection script so others can use it on their sites too. Download here, and read the included readme.txt file too before using. It explains what is what, and what its difference is compared to similar solutions found elsewhere. You can see the work this detection script does in our mobile statistics (OSNews serves about 1500 pageviews per day on non-desktop browsers via this script).
Thread beginning with comment 311382
To read all comments associated with this story, please click here.
Comment by deathshadow
by deathshadow on Fri 25th Apr 2008 02:29 UTC
deathshadow
Member since:
2005-07-12

Sweet to see all of these documented - that part is going to be handy...

But the code - mein gott. You've got no short-circuit eval there, so if the first condition is true all the rest will still run even if you KNOW they aren't needed. Likewise the 'traps' at the end should be done FIRST with an exit to reduce the execution time. The key to good code is to use as FEW if's as possible, not to chain them end to end like that...

The function wrapper 'istr' is also a bad thing - since it's a one to one wrap. Those handful of bytes you saved by not typing stristr each and every time is absurdly offset by the overhead of stack manipulation and 'calls' to the intermediate function.

Or just the simple wasted overhead of using 'parsing model' double quotes instead of the faster/cleaner single quotes. (remember in languages like php there's a difference, with one using more overhead and more complex parsing code)

Since you're only setting one value, wouldn't it make more sense to put this all into an array, putting the LONGER ones that would be false positives FIRST in the list, followed by a short-circuit eval.

This probably should also be a function in a Library returning the value instead of using a global. Remember programming 101 - globals bad. (though a necessary evil at times)

Other optimizations could also be possible - for example you could speed it up even more by putting the most likely/common results first.

Edited 2008-04-25 02:45 UTC

Reply Score: 4

RE: Comment by deathshadow
by Eugenia on Fri 25th Apr 2008 02:35 in reply to "Comment by deathshadow"
Eugenia Member since:
2005-06-28

No, there was a reason why I wanted each keyword on its own line: so I can identify keywords and add a set of special variables if a specific keyword is set, and then later, modify my HTML code based on these variables. Because you see, there were some phones or some versions of browsers that required special cHTML writing because of bugs, so I had to exclude them like that. You don't see that in the version of the code I provided, but the original we used on osnews has those exceptions in. It was much more readable for me to do it like this instead of putting everything into arrays.

Edited 2008-04-25 02:36 UTC

Reply Parent Score: 1

RE[2]: Comment by deathshadow
by prickett on Fri 25th Apr 2008 03:03 in reply to "RE: Comment by deathshadow"
prickett Member since:
2007-04-03

Please show us the code your talking about, because whilst the gesture of offering this work is a great thing, the programming itself is extremely amateurish and we could help you improve and at the end of the day everybody wins.

Edited 2008-04-25 03:04 UTC

Reply Parent Score: 1

RE[2]: Comment by deathshadow
by ohbrilliance on Fri 25th Apr 2008 03:36 in reply to "RE: Comment by deathshadow"
ohbrilliance Member since:
2005-07-07

I understand that there's a historical reason to separate the ifs rather than running the more efficient 'if/else', but since the code no longer needs to function in that manner, why not convert all but the first clause into an 'else' statement? Really seems baffling that you've left it as-is. Or have I misunderstood something?

Reply Parent Score: 2

RE: Comment by deathshadow
by kamil_chatrnuch on Fri 25th Apr 2008 14:03 in reply to "Comment by deathshadow"
kamil_chatrnuch Member since:
2005-07-07

just a question: would it be a problem to post the improved version?

Reply Parent Score: 2