Optimizing Code Question, multiple If Else Statements PHP

I'm a pretty terrible coder, but am trying to get better. I recently coded something that works, hooray! But I feel like there must be a cleaner way. Object is:

If GET variable exists use it as the cookie value. If the Variable doesn't exist use a default value. If the Cookie already exists use that value instead of a new one.

if (!isset($_COOKIE['id'])) {
  if (!isset($_GET[id])) {
    $cookid="115";
  } else {
    $cookid= $_GET["id"];
  } 
  setcookie("id", $cookid , ti开发者_JAVA百科me() + 31536000);
} else {
  $cookid= $_COOKIE['id'];
}

---

Here's how I'd have written it:

if(isset($_COOKIE['id'])) {
  $cookid = $_COOKIE['id'];
} else {
  $cookid = isset($_GET["id"]) ? $_GET["id"] : "115";

  setcookie("id", $cookid, time() + 31536000);
}

First of all, according to some conventions you should test for the positive case (isset(...)), not the negative (!isset()). I think it makes for more readable code, so I've switched it around.

Secondly, I've used the ternary operator (condition ? expr1 : expr2) to eliminate an if/else block, which is acceptable because you were just using the if/else block to decide which of two values to assign to the variable. The ternary operator should be used with caution, though, because if overused it can make code less readable.

Thirdly, it's my preference to use curly braces even for one-line if/else blocks, but at the very least I think you should use curly braces for the else if you use them for the if.

And lastly, just a readability note: Try to be consistent with your whitespace. On line four you don't have any spaces around the =, but on lines six and ten you have a space after it, but not before. For readability it's almost always preferable to have whitespace on both sides of an operator.

Oh, and $_GET["id"] is correct; $_GET[id] is not. It (correctly) throws a warning if you have your error reporting level high enough (and you should usually develop with error_reporting(E_ALL); so you see them).

---

if (isset($_GET['id'])) {
  $cookid = (int) $_GET['id'];
  setcookie('id', $cookid, time() + 31536000);
} else if (isset($_COOKIE['id'])) {
  $cookid = (int) $_COOKIE['id'];
} else {
  $cookid = 115;
  setcookie('id', $cookid, time() + 31536000);
}

---

I would encapsulate it into a function that can be easily re-used:

<?php
/**
 * If GET variable exists use it as the cookie value.
 * If the Variable doesn't exist use a default value.
 * If the Cookie already exists use that value instead of a new one.
 * 
 * @param string $name name of cookie / get variable
 * @param string $default default value
 * @return string the value.
 */
function cookie_get_value($name, $default) {
    if (isset($_COOKIE[$name])) {
        $value = $_COOKIE[$name];
    } else {
        $value = isset($_GET[$name]) ? $_GET[$name] : $default;
        setcookie($name, $value , time() + 31536000);
    }
    return $value;
}

echo cookie_get_value('id', '115');

In the end you need to do the if's you'd need to do. It's not always easy to make the own code better, however if you put it into a function of it's own it's easier to modify.

---

I believe you could use $_REQUEST, since it already has $_POST, $_GET and $_COOKIE in it

if (!isset($_REQUEST['id']))
{
   $cookid = '115';
   setcookie("id", $cookid , time() + 31536000); 
}
else
  $cookid = $_REQUEST['id'];