Session flash class

Question

I've created this class, for flashing variables like validation errors or success messages. But I'm not certainly sure about it.

<?php

namespace Basalt\Http;

class Flash
{
    const MESSAGE_PREFIX = 'flash_';

    /**
     * Return flashed message.
     *
     * @param string $name Flash message name.
     * @param bool $delete Should it be deleted?
     * @return mixed
     */
    public function get($name, $delete = true)
    {
        if (isset($_SESSION[$name = self::MESSAGE_PREFIX . $name])) {
             $value = $_SESSION[$name];

            if ($delete) {
                unset($_SESSION[$name]);
            }

            return $value;
        }

        return null;
    }

    /**
     * Flash value.
     *
     * @param string $name Flash message name.
     * @param mixed $value Value to flash.
     * @return void
     */
    public function flash($name, $value)
    {
        $_SESSION[self::MESSAGE_PREFIX . $name] = $value;
    }
}

Answer 1

In general functionality is fulfilled but I'd improve it a bit. Below you will find incremental (further ones will contain previous improvements) list of improvements.

1. Split get to two methods, get and peek, the former one would delete the returned flash message and the latter only return it. Optional parameters that affect method logic suggests that the logic should be split.

2. You should define a method to check whether flash is set then you can use it in get, peek and gives more utility to user:

   public function has($name)
   {
       return isset($_SESSION[self::MESSAGE_PREFIX . $name];
   }
   

3. Name for flash method does not really tell what it does, imho better would be simply set this also makes direct opposition to get. In overall it is more coherent naming scheme.

4. You might want to make MESSAGE_PREFIX as a private static variable, class constants are accessible from outside and this gives ability to mess up directly on the superglobal $_SESSION variable. Also it would be safer to have the prefix more "vendor" specific it will help to avoid conflict with other libraries / direct operations on $_SESSION i.e.

   private static $MESSAGE_PREFIX = 'Basalt_flash';
   

5. You are "polluting" $_SESSION with many values, it would be better to scope your flashes inside one array. The get method would look this way

   public function get($name)
   {
       if ($this->has($name)) {
           $value = $_SESSION[self::$MESSAGE_PREFIX][$name];
   
           unset($_SESSION[self::$MESSAGE_PREFIX][$name]);
   
           return $value;
       }
   
       return null;
   }
   

6. It is a good practice / pattern to return early, this improves readability and keeps indentation at low level i.e. for get method

   public function get($name, $delete = true)
   {
       if (!$this->has($name)) {
           return null;
       }
   
       $value = $_SESSION[self::$MESSAGE_PREFIX][$name];
   
       unset($_SESSION[self::$MESSAGE_PREFIX][$name]);
   
       return $value;
   }
   

The final result would look this way:

<?php

namespace Basalt\Http;

class Flash
{
    private static $MESSAGE_PREFIX = 'Basalt_flash';

    public function has($name)
    {
        return isset($_SESSION[self::$MESSAGE_PREFIX][$name]);
    }

    /**
     * Return flashed message.
     *
     * @param string $name Flash message name.
     * @return mixed
     */
    public function get($name)
    {
        if (!$this->has($name)) {
            return null;
        }

        $value = $_SESSION[self::$MESSAGE_PREFIX][$name];

        unset($_SESSION[self::$MESSAGE_PREFIX][$name]);

        return $value;
    }

    public function peek($name)
    {
        if (!$this->has($name)) {
            return null;
        }

        return $_SESSION[self::$MESSAGE_PREFIX][$name];
    }

    /**
     * Flash value.
     *
     * @param string $name Flash message name.
     * @param mixed $value Value to flash.
     * @return void
     */
    public function set($name, $value)
    {
        $_SESSION[self::$MESSAGE_PREFIX][$name] = $value;
    }
}

Or instead of reinventing the wheel you can use something that is already made, is well tested, documented and supported like symfony HttpFoundation and its Session subsystem.