0

I am building a script that will process a URL, and will modify it according to various conditions.

I'm not sure what is the best practice to do this; for example, right now, under "Partner A" section, if the category is different than "premiuim", my final_url variable will remain empty. I guess I can make a condition that will check if it's empty and in that case it should apply to it the value of $url3, but it doesn't feel smart enough:

<?php

$url = "http://www.default.com"; // default URL

$url = "something"; // this part will come from a database

$id="something"; // some value to insert later into URL

// the following rules will be followed according to various parameters not defined in this sample code

if($partner == "A") { // Partner A rule:

    $url2 = str_replace('777', '999', $url); // Inital Change

    $url3 = str_replace('?location', '?id=' . $id . '&location', $url2); // Add id, anytime ?location shows up

    if($category == "premium") { // premium category rules:

        $re = "/(?<=TID%3D)\\d+/";  // String to Replace in URL (digits)
        $newtid = "4000"; // Default New TID

                if($special == "yes") { $newtid = "8000";} // special TID value

        $final_url = preg_replace($re, $newtid, $url3); // replace the TID

    }

};


if($partner == "B") { // Partner B rule:

$final_url = str_replace('status=1', 'config=' . $id . '&status=1', $url); //Add id, anytime status=1 exists in url
};

?>  

<p>Final URL: <?php echo $final_url; ?></p>

Also, I'm not sure that the whole structure is the optimal one, in general.

EDIT:

updated code:

if($category == 'A') {
    // A category rules:
    $url2 = str_replace('aaa', 'bbb', $url); // Global rule #1
    $url3 = str_replace('?url', '?id=' . $id . '&url', $url2); // Glbal rule #2
    $final_url = $url3; // Final URL unless following cases occur

    if($offer == 'Special') {
        //category is A and offer is special

        $re = "/(?<=TID%3D)\\d+/";  // replace rule for current case
        $tid = "123"; // Default New TID value for current case
        $final_url = preg_replace($re, $tid, $url3); // Final URL for this case unless following case occur

        if ($discount == '10') {
            // category is A, offer is special, and Discount is 10

            $re = "/(?<=TID%3D)\\d+/";  // replace rule for current case
            $tid = "999"; // Special TID value for current case
            $final_url = preg_replace($re, $itd, $url3); // Final URL for this case
        }
    }
} 
rockyraw
  • 1,125
  • 2
  • 15
  • 36

2 Answers2

1

This should simplify the code:

if($category == 'A') {
    // A category rules:
    $final_url = str_replace(array('aaa', '?url'), array('bbb', '?id=' . $id . '&url'), $url);
    $re = "/(?<=TID%3D)\\d+/";  // replace rule for current case
    if($offer == 'Special') {
        //category is A and offer is special
        $tid = "123"; // Default New TID value for current case
        if ($discount == '10') {
            // category is A, offer is special, and Discount is 10
            $tid = "999"; // Special TID value for current case
        }
       $final_url = preg_replace($re, $tid, $url3);
    }
}

Your regex is the same so you don't need to define it twice. The str_replace can also take multiple search fields and replaces values.

chris85
  • 23,846
  • 7
  • 34
  • 51
0

One efficient way to approach this might be a switch/case statement:

switch (TRUE) {

case ($partner == 'A') :
/* rules; */

if ($category == 'premium') {
/* rules; */
}

else {
/* rules; */
}

/* rules; */

break;

case ($partner == 'B') :
/* rules; */
break;
}

=====

Original suggestion:

switch (TRUE) {

case (($partner == 'A') && ($category == 'premium')) :
/* rules; */
break;

case ($partner == 'A') :
/* rules; */
break;

case ($partner == 'B') :
/* rules; */
break;
}

=====

Modified suggestion:

switch (TRUE) {

case (($partner == 'A') && ($category == 'premium')) :
/* rules; */

case ($partner == 'A') :
/* rules; */
break;

case ($partner == 'B') :
/* rules; */
break;
}
Rounin
  • 27,134
  • 9
  • 83
  • 108
  • isn't there a default action to be executed if no condition is met? – danidee Jan 02 '16 at 16:08
  • Yes, you can add one. But it's not compulsory. – Rounin Jan 02 '16 at 16:08
  • would the rules of `($partner == 'A')` apply into the case of `case (($partner == 'A') && ($category == 'premium'))`, unless they are overridden by other rules? for example, under `case (($partner == 'A') && ($category == 'premium'))`, can I only write the line `$newtid = "8000";` and it will work just like original code? or do I have to repeat all general rules under the special case as well? – rockyraw Jan 02 '16 at 16:17
  • You _can_ write only the line `$newtid = "8000";` under `case (($partner == 'A') && ($category == 'premium'))` - but if you do, be sure to remove the `break;` immediately below it, so that the PHP parser continues onward to check the next case ( `($partner == 'A')` ) and doesn't break out of the structure. – Rounin Jan 02 '16 at 16:28
  • weird, when I take off the `break` the code treat the next condition as if it is true even in cases it isn't... – rockyraw Jan 02 '16 at 16:42
  • You mean when `$partner == 'B'`, the parser is still following the rules for `$partner == 'A'`? – Rounin Jan 02 '16 at 16:48
  • no, I mean that when `$partner == 'A'` BUT `category` isn't `premium`, script still applies rules from premium category. – rockyraw Jan 02 '16 at 16:53
  • I took [some major heat](http://stackoverflow.com/questions/34568364/switch-case-without-a-break-doesnt-check-the-cases-properly) for trying to use switch/case for this. – rockyraw Jan 02 '16 at 17:15
  • can you add a sample of such `if/else` inside a `case` without a `breal` to your answer? – rockyraw Jan 02 '16 at 17:26
  • I've updated the post above to show an `if/else` inside a `case`. – Rounin Jan 02 '16 at 17:31
  • modified and original looks like the same code, is that a mistake? – rockyraw Jan 02 '16 at 18:26
  • The single difference is the missing `break;` immediately below the first `case` (see my comment which begins "_You **can** write only the line..._" above). – Rounin Jan 02 '16 at 18:38