This should work. I added the in_array so they can only include certain things.

added a check to see if $_GET['id'] is set. Also added in a else statement to display news as the default page you see when visiting index.php.

Avoid array key does not exist errors and default to the news pages if no page id, page id is invalid, or the file is not found.

wow you definitely refactored that down to size. :)

I cannot get my php codes to highlight.. :( I must need need to include the opening and closing tags I will try again.

Guys, you didn't refactor the code, you just made it more complicated.

Switch statement is perfectly fine for this. Why do you need to add "allowable_pages"? Or regular expressions?!

Just leave it how it is.

Hi, I know this might be a bit simple but its better practice to use single-quotes rather than double if all you want to do is deal with a string datatype. That is because having double-quotes causes php to parse the string looking for any variables to deal with. So having single-quotes saves php from doing this, hence a slight(so slight that it might not really matter but hey faster is faster...) preformance boost.

@No: Yes is does require a more advanced knowledge of php to understand, and I wouldn't expect a beginner to understand it easily. If it was me it would be all one ternary operation.

A switch is a good solution and leave it was it was would be a good solution, except add a check to ensure $_GET['id'] is set so you can run with notices turned.

Hi, I agree with Mike Cochrane, his solution is rebust, effective and concise. Like the use of ternary operator (its too under valued ...). What I've done is taken his solution (slight change but in essence the same) and wrapped it up in a helper function ready for reuse. The helper.php file contains an array of pages and a default page and the helper_include() function. All you need to do is include the helper.php file and then call the help_include() function passing it the name of the URL variable in the case 'id'. You just need to add 2 lines of code every new page you wish to create.

"Yes is does require a more advanced knowledge of php to understand, and I wouldn't expect a beginner to understand it easily. If it was me it would be all one ternary operation."

What a load of bullocks, the switch statement is really easy to understand and does exactly what it says, you can just read it and understand it even if you don't know PHP.

The switch statement also allows you to have different pages -> php files, it's secure and by far the fastest one posted here.

Why you need to run file_exists is beyond my understanding (or even a "helper.php"? helping to obfuscate?) and the "default:" is a much nicer solution than having if's.

You really need to have a look at "Keep It Simple", this code is OBVIOUSLY related to a small project with a handful of pages, and this IS how you do it in this case. Keep posting your over-engineered pseudo smart code, nothing beats the switch statement here.

What happens when a user types the following URL and tries to go to it:
[www.somesite.com/index.php?id=some-page-that-does-not-exist]

Wow, you guys are making it a bit complicated. ;) Here's what I'd do... No functions, no unne

@Moody: You'll need to global $aryPages; in the function as they're in different scopes.

@Don Wilson: Good point about the strtolower and trim.

@No: Good point about the file_exists. I'd drop that out of my code and assume that I'd only add pages that exist to the array.

"What happens when a user types the following URL and tries to go to it: "

default: [...] include "news.php";

"Wow, you guys are making it a bit complicated. ;) Here's what I'd do... No functions, no unne"

You are making it complicated. Why do you need to use preg_replace? strtolower? Why do you want to clean the user input? If he fiddles with the URL he doesn't need to be rewarded. Do you really want Google to pick up HOME HoME HOmE etc?

Not every code needs to be refactored.

Not ONE refactoring on this page is:

1. Faster
2. Safer
3. Easier to maintain
4. Easier to read
5. Easier to figure out when you come back to it after a couple months

"No" - you do not want to keep the user from using your site. Using preg_replace allows you to remove unnecessary characters ("..", ".", "/", etc).

"Why do you want to clean the user input?"
Why wouldn't you clean an input that could be changed so easily? Especially something that you'll utilize when managing files/databases/etc.

1. Speed of code is a completely unnecessary worry in this code sniplet.
2. I made it far safer than any other modification that I've seen before.
3. It's five lines of code, not a thousand line class.
4 + 5. I don't think you have to worry too much about figuring out 5 lines of code a few months ago.

Fixing typos

"Why wouldn't you clean an input that could be changed so easily? Especially something that you'll utilize when managing files/databases/etc."

A switch statement is a 100% bullet proof. There is no need to clean the input.

"2. I made it far safer than any other modification that I've seen before. "

Irrelevant, it doesn't get "safer" than a switch statement. All other solutions posted here can however suffer from a potential buffer overflow.

"3. It's five lines of code, not a thousand line class. 4 + 5. I don't think you have to worry too much about figuring out 5 lines of code a few months ago."

Exactly, so why refactor it?

And just for No: the switch with a check to ensure $_GET['id'] exists first. I like to be able to run my code with the highest level of debugging on and have it run clean.

I would contest "3. Easier to maintain" and say that adding a single option to an array is easier to maintain then copying 3 lines of code and changing the value twice.
"2. Safer" both are the same 'safeness'. There is no room to exploit either - on the assumption that php's functions are safe from overflows.

Mike Cochrane, would there even be a need for the "isset" when dealing with the switch statement? If you leave it out and go to index.php the "ID" is not set so the switch would use the "default:" value and direct you to index.php?id=news as you must already know. So by adding an "isset" your basically stopping the default from working, but then you added in an else statement to include a default page if "ID" is not set....so your kind of making more work for your self. At least thats how it seems in my eyes.

@NO - I agree to some extent that the original Switch statement is fine as it is, but when dealing with loads of pages to include, this might slow the switch down noticeably not to mention you're neat little switch would end up being a big blob of code that takes up most of the page. Mike Cochrane's code (that's 2 lines long) ultimately makes the code neater and easier to edit (1 line to edit as apposed to 2 in the switch). I'd probably go with Mike's. I don't know how much of a difference the speed would be so thats still up for debate I guess.

If you develop with error_reporting set to E_ALL (and I recommend you do) you will receive a
Notice: Undefined index: id in test.php on line 2
If you just assume that it is set and don't explicitly check.

I did a quick performance test. Using the two functions below - eliminating the file operations and returning the string instead. Run once timed, change function, timed, changed back, timed, changed to v2 again, timed.
For two pages, the switch took 49.9% of the time of my code - switch wins by heaps (15,000 calls - two runs avg of both).
For 101 pages, the switch took 49.5% of the time of my code - (255,000 calls - one run each)

So there you have it. The switch is twice as fast as my code. Take your pick :-)

FYI in 101 page test:
v3 was 107% of v2 (the ternary construct is faster in this case).
v4 was 52% of v2 (in_array is the killer in v2).
v5 was identical to v2 (v2 with array_search($_GET['id'], $pages) !== false instead of in_array)

Interesting, So I guess even though a switch is longer and has more places to edit its still overall faster?

I see no reason to put this on multiple lines.
By placing small switches on the same line it is easier to see the trends.

I do agree with Anonymous, although you can obviously do the isset() and default in one single, readable statement. Also, I agree with mister No, don't overcomplicate thinks if it's not necessary. This is not code to refactor.

berry___, instead of the first line setting $page, put in a default: at the bottom of the switch.

so... what way should i do it (not sure what changes are correct and what ways actually work)

also here is code that i currently use,

after reading through all of your code and comments, i guess i need to make it more secure

They all work, but it appears a switch is the fastest (at least thats what Mike Cochrane's test showed). So you could use any method you want.

@Don Wilson - Wouldn't you want the default to be at the top? This way if someone doesn't go to a specific page, php doesn't have to go through the whole list.

in_array is slow, but you can still use an array. All you have to do is use array keys instead of array values. The advantage with arrays remains...there's slightly less repetition here than with a switch statement. This should be faster than in_array implementations:

I like how clean the Switch statement is, but if you want the absolute best performance; use the if/elseif/else statement and triple equals.

Or you could do this:

@ EllisGL

Warning with your code its possible to hack the webserver.
I had this in my apache log (see code)

If you want to use that code you must run php in safe mode and/or have apache mod_secure

The shortest option and the best option

Peacefire.org$_GET['id'].".php";

So, in my blog, I look for opportunities to present perspectives that are often censored. ,

очень занимательно было почитать

А еще говорят, что в инете не найдешь по-настоящему интересную информацию!