Cookie Management Class

I wrote this class to make cookie management easier.

I know it makes use of serialize(), but I have no intention of storing very much data in the cookie, and it seemed a cleaner solution than, say, the JSON functions (like, even if a class implements the JSONSerializable interface, it doesn't retain the class name by default, etc.). serialize() seemed more robust, although if there's a way to compress the output without it losing its integrity, please share. Probably the worst pitfall is that, if the cookie is messed with, unserialize() will trigger a notice and who knows what else -- but I don't feel I need to cater to the user experience of users who mess with cookies. (Although if the cookie is maxed out at a number of bytes, like 4,096, and it truncates, that could be an issue....)

I am in the process of writing a class to put on top of this one for managing user sessions implementing Charles Miller's solution.

I decided against implementing exceptions because it was simple enough to just return constants.

```

* @version 1.0
* @package Miller
* @subpackage Utils
*/

namespace Miller\Utils;

if (!function_exists('array_copy')) {
/**
* array_copy Properly and recursively clones an array
* @param array $source The original array
* @return array The clone
*/
function array_copy ($source) {
$return = array ();
$keys = array_keys($source);
$values = array_values($source);

for ($i = 0; $i name = (string) $name;

if ($expiry === self::SESSION) {
$this->expiry = $expiry;
}
else if (is_numeric($expiry)) {
$this->expiry = time()+$expiry;
}
else {
$this->expiry = strtotime($expiry);
}

$this->path = (string) $path;
$this->domain = (string) $domain;
$this->secure = $secure === null ? isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] : (bool) $secure;
$this->httponly = (bool) $httponly;

First off, I've posted a fairly length answer on the subject of request classes, which can be found here. Because you state you're thinking of writing a Session class on top of this one, I'm posting a link to my answer here.

In that answer, I discuss what you should keep in mind when doing so. For example: using setters and getters, sanitize all request data etc...

Then, as I said in the comment, I noticed you're using the ?> closing tag, which is optional. Omitting it, especially in those files that will be include or require-ed is considered good practice. It's, amoungst other things, to avoid excess whitespace. Read more here

Another part of good practice is to write the doc-blocks just before the method definition, not in the function body. As soon as you start using the ReflectionClass (especially the getDocBlock method), you'll see the value of this. Even if you don't use it, other libs might (like Doctrine to name one, not unimportant example)

You're also defining a method called __isset. The double underscore would suggest this is a magic-method, that enables the user of this class to write something like:

if (isset($instance['cookieName']))

And that php will convert that into if ($instance->__isset('cookieName')). That's not the case. The magic isset method only works when you write isset($instance->cookieName), which, to all intents and purpouses still is a different syntax.

If I were in your shoes, I'd implement a Travesable interface, or the ArrayAccess interface. read through the examples, and you'll see array-like usage of isset is possible thanks to this interface.

The last remark I'd like to make is the fact that your file is called Cookie.class.php, so I'd expect its contents to be just a class definition. It isn't, though: you're defining a function, too. That's just not nice. I'd suggest you read, and make sure to comply with the unofficial PHP-FIG standards. Though not official, all major players (check list here) subscribe to these standards. If your code complies, too, autoloaders from any of the major frameworks will just work with your classes.

While I'm on the subject of autoloaders: implement them, use them, rely on them. If your classes are defined according to the aforementioned standards, you can do away with all those pesky require's. Even if you do need the occasional require: use the safer require_once variant. Sure it's a tad slower, but at least it's safe.

Oh, and if you still need that array_copy in the Miller\Utils namespace, you could use a static method, since PHP statics are just globals in drag, or declare a global function and call it with a leading \.

Still, look at where you're using it:

public function __construct ($name, $expiry = self::WEEK, $path = '', $domain = '', $secure = null, $httponly = false)
{
    $this->name = (string) $name;
    if ($expiry === self::SESSION) {
        $this->expiry = $expiry;
    }
    else if (is_numeric($expiry)) {
        $this->expiry = time()+$expiry;
    }
    else {
        $this->expiry = strtotime($expiry);
    }

    $this->path = (string) $path;
    $this->domain = (string) $domain;
    $this->secure = $secure === null ? isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] : (bool) $secure;
    $this->httponly = (bool) $httponly;

    $this->properties = array ();
    $this->internalProperties = array ();
    if (isset($_COOKIE[$this->name])) {
        $properties = unserialize($_COOKIE[$this->name]);
        $this->internalProperties = array_copy($properties);
        $this->properties = array_copy($properties);
    }
}

You're clearly using it to set properties inside the class, so why isn't this function a private method?

Moving on to some actual code review: Your constructor is doing way to much work. Besides, when I create a cookie, I'd like to be able to set the data when I choose, not passing it all to the constructor. I'd like to use a Cookie class to check if a given cookie exists, and if it does, check when it expires, if it doesn't I'd like to create it and maybe determine its expiration date/time later on. Implement setters and getters for that.
Also use these setters to sanitize whatever data is being passed to the constructor. Sure:

$this->name = (string) $name;

Assures you that the name property is bound to be a string. Good, but what if I passed an object. I'm not going to get an exception thrown, though I clearly passed an argument that is a good reason to throw an InvalidArgumentException. Don't just cast to the types you want, check if the arguments passed are:

• Castable to the type without loss of data

• Valid types in the first place

By the second I mean that, if I were to accidentally pass an instance of PDO as name, I should be notified of the error in my code. Basically: your class is too reliant on magic-methods, it allows overloading of properties, which is terrible, especially with request objects (the request sh

StackExchange Code Review Q#30895, answer score: 2