Better Capture to Session Affiliate Id from GET in PHP

Question

Can someone help me clean this up and make it more logical? I'm fried right now and can't seem to get a good line of code written :)

I'm trying to capture affiliate id from urls like ?aid=3056677. The idea is IF aff id is set in GET that takes precedence, the session and finally cookie with least. Also, we don't want to set an aff id that does not exist.

Do you know of a more tried and true method of doing this?

session_start(); // start session

// affiliate id
$g_aid = (isset($_GET['aid']) && $_GET['aid'] != '') ? trim($_GET['aid']) : false;
$s_aid = (isset($_SESSION['aid']) && $_SESSION['aid'] != '') ? trim($_SESSION['aid']) : false;
$c_aid = (isset($_COOKIE['aid']) && $_COOKIE['aid'] != '') ? trim($_COOKIE['aid']) : false;

if($g_aid !== false) // use get if set
  $aid = $g_aid;
elseif($s_aid !== false) // next use session if get not set
  $aid = $s_aid;
elseif($c_aid !== false) // cookie
  $aid = $c_aid;
else
  $aid = ''; // leave it empty

// if $aid is set is it in the $affiliates array?
//If not use the first key from that array
$aid = (isset($affiliates[$aid])) ? $aid : key($affiliates);

// save it and set it
// (maybe shouldn't be done if already stored?
setcookie('aid', $aid);
$_SESSION['aid'] = $aid;

Answer 1

session_start();

// checks if a field is valid
function isValid($aid) {
    return (!empty($aid) && trim($aid) != '');
}

// set the affiliate ID
$aid = isValid($_GET['aid'])     ? $_GET['aid'] :
       isValid($_SESSION['aid']) ? $_SESSION['aid'] : 
       isValid($_COOKIE['aid'])  ? $_COOKIE['aid'] :
       '';

// use first key from array if aid not set
if (!isset($affiliates[$aid])) $aid = key($a);

// save and set 
setcookie('aid', $aid);
$_SESSION['aid'] = $aid;

Answer 2

1. Why would you test for session and cookie, in case you have a valid affiliateID from the $_GET array? ==> Make it progressive so that session is only checked, if no GET was found and cookie is only checked if no session was found.

2. Don't repeat the validation of the affiliateID. ==> Write a validate function and reuse it, you might want to add more rules later on.

3. Use curly brackets to make your code more readable

4. $aid or $aff are BAD variable names, $affiliateID instead is a GOOD one! You don't win anything for writing short variables names but you win a lot with writing self-explanatory code.

Bad example, doesn't talk

if (validate($aff)) 

Good example, talks to you

if (isValid($affiliationID))

So my proposal for change of the core components:

if (isValid($_GET['aid']))
{
    $affiliationID = trim($_GET['aid'];
}
else if (isValid($_SESSION['aid']))
{
    $affiliationID = trim($_SESSION'aid'];
}
else if (isValid($_COOKIE['aid']))
{
    $affiliationID = trim($_COOKIE['aid'];
}
else
{
    throw new Exception('No affiliation ID defined');
}

function isValid($affiliationID)
{
    if (empty($affiliationID))
    {
        return false;
    }
    else
    {
        return true;
    }
}

Answer 3

Thanks guys this is looking better and better. One point that might clarify for you, is that IF an aff id is given in GET it MUST be a valid one that exists before we possibly wipe out someone else's aff id. Money is involved with each transaction and we want an affiliate to get credit for as long as possible.

Regarding empty it's not too useful since whitespace fools it. So unless you trim before using it, I feel it's not accurate enough. So I don't know about the empty for the GET. It's ok for the others because we've already checked them.

Here's what I have so far from your help (does the complex ternary here break when it finds true? I don't want it to keep executing the line):

session_start(); // start session

  $aid = !empty($_GET['aid'])     ? trim($_GET['aid']) :
         !empty($_SESSION['aid']) ? $_SESSION['aid'] : 
         !empty($_COOKIE['aid'])  ? $_COOKIE['aid'] :
         '';

  // use first key from array if aid not set
  if(!isset($a[$aid])) $aid = key($a);

  if(!isset($_SESSION['aid']) || $aid != $_SESSION['aid'])
  {
    setcookie('aid', $aid);
    $_SESSION['aid'] = $aid;
  }