Skip to main content
added 783 characters in body
Source Link
mseancole
  • 6.1k
  • 1
  • 15
  • 27

Disclaimer: I do eventually answer your question, there are just some other things I wished to point out.

GET vs REQUEST

First of all, I would consider using $_GET instead of $_REQUEST. I'm assuming its get your using judging by the data you are passing. It is generally a better idea to use the specific array you need if you know it rather than one that could potentially contain data from another source. For example, should you ever create a form with a path input, it will supersede any get path. But this isn't even necessary, the remote user can change that value should they wish. However, if you were to call them separately $_GET[ 'path' ] and $_POST[ 'path' ] you will be able to use them both together without losing any information and they are less likely to be hijacked (minus get, its pretty easy to hijack that).

Filter Input

Array Shift

Order of Logic

Instead of checking if the value of $urlSegments is empty, I would set it first then check it. A couple of reasons for this. What if "path" starts with a forward slash "/"? I believe explode will produce an empty element at the very beginning. Unless this is the desired result, it could cause errors. Also, it removes the need for those magic numbers, i.e.($urlSegments[0]) Yes its obvious where it came from and why you are using it, but it is a bad habit and is not extendable.

Here's the Better Solution: Only one parameter

EditHere's the Better Solution: Multiple parameters 

I just realized that $urlSegments could very likely be longer than 3 elements. In this case, I would filter and pad it like I did above, shift the first two elements into their respective variables then set $parameters to whatever remains of $urlSegments. Padding it first ensures that each variable has a default value of an empty string.

Shorthand

Finally, it is rarely a good idea to use shorthand in PHP. If I did not know what you were trying to accomplish, or had a slightly older version of PHP, I would think that $urlSegments = []; was syntactically wrong. Yes it works, but you could run across problems in the future. Not to mention its just easier to read $urlSegments = array();

Regex

That should clean up your code considerably. I would hesitate to suggest any regex, mainly because I find it extremely difficult to read.

Edits

Edited a few times to clean up post and add the second "Better Solution".

I do eventually answer your question, there are just some other things I wished to point out.

First of all, I would consider using $_GET instead of $_REQUEST. I'm assuming its get your using judging by the data you are passing. It is generally a better idea to use the specific array you need if you know it rather than one that could potentially contain data from another source. For example, should you ever create a form with a path input, it will supersede any get path. But this isn't even necessary, the remote user can change that value should they wish. However, if you were to call them separately $_GET[ 'path' ] and $_POST[ 'path' ] you will be able to use them both together without losing any information and they are less likely to be hijacked (minus get, its pretty easy to hijack that).

Instead of checking if the value of $urlSegments is empty, I would set it first then check it. A couple of reasons for this. What if "path" starts with a forward slash "/"? I believe explode will produce an empty element at the very beginning. Unless this is the desired result, it could cause errors. Also, it removes the need for those magic numbers, i.e.($urlSegments[0]) Yes its obvious where it came from and why you are using it, but it is a bad habit and is not extendable.

Here's the Better Solution

Edit: I just realized that $urlSegments could very likely be longer than 3 elements. In this case, I would filter and pad it like I did above, shift the first two elements into their respective variables then set $parameters to whatever remains of $urlSegments. Padding it first ensures that each variable has a default value of an empty string.

Finally, it is rarely a good idea to use shorthand in PHP. If I did not know what you were trying to accomplish, or had a slightly older version of PHP, I would think that $urlSegments = []; was syntactically wrong. Yes it works, but you could run across problems in the future. Not to mention its just easier to read $urlSegments = array();

That should clean up your code considerably. I would hesitate to suggest any regex, mainly because I find it extremely difficult to read.

Disclaimer: I do eventually answer your question, there are just some other things I wished to point out.

GET vs REQUEST

First of all, I would consider using $_GET instead of $_REQUEST. I'm assuming its get your using judging by the data you are passing. It is generally a better idea to use the specific array you need if you know it rather than one that could potentially contain data from another source. For example, should you ever create a form with a path input, it will supersede any get path. But this isn't even necessary, the remote user can change that value should they wish. However, if you were to call them separately $_GET[ 'path' ] and $_POST[ 'path' ] you will be able to use them both together without losing any information and they are less likely to be hijacked (minus get, its pretty easy to hijack that).

Filter Input

Array Shift

Order of Logic

Instead of checking if the value of $urlSegments is empty, I would set it first then check it. A couple of reasons for this. What if "path" starts with a forward slash "/"? I believe explode will produce an empty element at the very beginning. Unless this is the desired result, it could cause errors. Also, it removes the need for those magic numbers, i.e.($urlSegments[0]) Yes its obvious where it came from and why you are using it, but it is a bad habit and is not extendable.

Here's the Better Solution: Only one parameter

Here's the Better Solution: Multiple parameters 

I just realized that $urlSegments could very likely be longer than 3 elements. In this case, I would filter and pad it like I did above, shift the first two elements into their respective variables then set $parameters to whatever remains of $urlSegments. Padding it first ensures that each variable has a default value of an empty string.

Shorthand

Finally, it is rarely a good idea to use shorthand in PHP. If I did not know what you were trying to accomplish, or had a slightly older version of PHP, I would think that $urlSegments = []; was syntactically wrong. Yes it works, but you could run across problems in the future. Not to mention its just easier to read $urlSegments = array();

Regex

That should clean up your code considerably. I would hesitate to suggest any regex, mainly because I find it extremely difficult to read.

Edits

Edited a few times to clean up post and add the second "Better Solution".

added 783 characters in body
Source Link
mseancole
  • 6.1k
  • 1
  • 15
  • 27

However, the cleanest way of all is to guarantee the length of $urlSegments, then use PHP's list(). So, you can use array_pad() to fill out the rest of $urlSegments should it be too short, otherwise list() will produce errors. If it is already of the proper size it will not do anything so you don't need to test it first. Now, to ensure that it doesn't already contain empty elements, that it didn't start with a forward slash, or have any double slashes, you should run it through array_filter(). Note: This will remove any element that could be considered "false" (0, '', null, -1, etc) so be careful here. This also means that if you were using empty or false parameters before, then it may not work the same way. Edit: This is if you use array_filter() without a callback, you could always make your own callback function to ignore certain values.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
list( $resource, $action, $parameters ) = $urlSegments;

Edit: I just realized that $urlSegments could very likely be longer than 3 elements. In this case, I would filter and pad it like I did above, shift the first two elements into their respective variables then set $parameters to whatever remains of $urlSegments. Padding it first ensures that each variable has a default value of an empty string.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
$resource = array_shift( $urlSegments );//default = '';
$action = array_shift( $urlSegments );//default = '';
$parameters = $urlSegments;//default = array( '' );

However, the cleanest way of all is to guarantee the length of $urlSegments, then use PHP's list(). So, you can use array_pad() to fill out the rest of $urlSegments should it be too short, otherwise list() will produce errors. If it is already of the proper size it will not do anything so you don't need to test it first. Now, to ensure that it doesn't already contain empty elements, that it didn't start with a forward slash, or have any double slashes, you should run it through array_filter(). Note: This will remove any element that could be considered "false" (0, '', null, -1, etc) so be careful here. This also means that if you were using empty or false parameters before, then it may not work the same way.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
list( $resource, $action, $parameters ) = $urlSegments;

However, the cleanest way of all is to guarantee the length of $urlSegments, then use PHP's list(). So, you can use array_pad() to fill out the rest of $urlSegments should it be too short, otherwise list() will produce errors. If it is already of the proper size it will not do anything so you don't need to test it first. Now, to ensure that it doesn't already contain empty elements, that it didn't start with a forward slash, or have any double slashes, you should run it through array_filter(). Note: This will remove any element that could be considered "false" (0, '', null, -1, etc) so be careful here. This also means that if you were using empty or false parameters before, then it may not work the same way. Edit: This is if you use array_filter() without a callback, you could always make your own callback function to ignore certain values.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
list( $resource, $action, $parameters ) = $urlSegments;

Edit: I just realized that $urlSegments could very likely be longer than 3 elements. In this case, I would filter and pad it like I did above, shift the first two elements into their respective variables then set $parameters to whatever remains of $urlSegments. Padding it first ensures that each variable has a default value of an empty string.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
$resource = array_shift( $urlSegments );//default = '';
$action = array_shift( $urlSegments );//default = '';
$parameters = $urlSegments;//default = array( '' );
Source Link
mseancole
  • 6.1k
  • 1
  • 15
  • 27

I do eventually answer your question, there are just some other things I wished to point out.

First of all, I would consider using $_GET instead of $_REQUEST. I'm assuming its get your using judging by the data you are passing. It is generally a better idea to use the specific array you need if you know it rather than one that could potentially contain data from another source. For example, should you ever create a form with a path input, it will supersede any get path. But this isn't even necessary, the remote user can change that value should they wish. However, if you were to call them separately $_GET[ 'path' ] and $_POST[ 'path' ] you will be able to use them both together without losing any information and they are less likely to be hijacked (minus get, its pretty easy to hijack that).

Take a look at PHP's filter_input(). It will mean you only have to type that $url variable once and has the added benefit of sanitizing your input.

$url = filter_input( INPUT_GET, 'path', FILTER_SANITIZE_STRING );
//returns false if not set so
if( $url ) {
    //etc...
}

PHP's array_shift() returns the value shifted, so there is no reason to specifically call that value, simply set the variable as you shift it.

$resource = array_shift( $urlSegments );

Instead of checking if the value of $urlSegments is empty, I would set it first then check it. A couple of reasons for this. What if "path" starts with a forward slash "/"? I believe explode will produce an empty element at the very beginning. Unless this is the desired result, it could cause errors. Also, it removes the need for those magic numbers, i.e.($urlSegments[0]) Yes its obvious where it came from and why you are using it, but it is a bad habit and is not extendable.

$resource = array_shift( $urlSegments );
if( ! empty( $urlSegments ) ) {
    if( ! empty( $resource ) ) {
        $action = array_shift( $urlSegments );
        if( ! empty( $action ) ) { $parameters = $urlSegments; }
    } else {
        //handle $url starting with a forward slash
        //this is where having this set up in functions would help
    }
}

Here's the Better Solution

However, the cleanest way of all is to guarantee the length of $urlSegments, then use PHP's list(). So, you can use array_pad() to fill out the rest of $urlSegments should it be too short, otherwise list() will produce errors. If it is already of the proper size it will not do anything so you don't need to test it first. Now, to ensure that it doesn't already contain empty elements, that it didn't start with a forward slash, or have any double slashes, you should run it through array_filter(). Note: This will remove any element that could be considered "false" (0, '', null, -1, etc) so be careful here. This also means that if you were using empty or false parameters before, then it may not work the same way.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
list( $resource, $action, $parameters ) = $urlSegments;

Finally, it is rarely a good idea to use shorthand in PHP. If I did not know what you were trying to accomplish, or had a slightly older version of PHP, I would think that $urlSegments = []; was syntactically wrong. Yes it works, but you could run across problems in the future. Not to mention its just easier to read $urlSegments = array();

That should clean up your code considerably. I would hesitate to suggest any regex, mainly because I find it extremely difficult to read.