Are the returns within my session function actually needed?

Question

I have the following object oriented class revolving about the PHP super global $_SESSION I shown the script to a fellow developer and he mentioned that all my returns are not necessary needed, whereas I believe that they are?

My code is as followed:

/*
*
*
*   This class will allow control of the super global $_SESSION within PHP
*       It's possible to import this class within websites which are already working with session variables. 
*   Currently the functionality provides:
*       Invoking a new session
*       Getting the current session stats
*       Setting a session key/value
*       Getting the session value
*       displaying the session array
*
*
*
*/

class Session { 
    protected $Session_Started = false; 

    public function __construct(){ 
        /*
        *
        *   If session is already invoked,  the constructor
        *   will switch the protected variable to true, and the session_start function will not be called
        *       If session is not already invoked, the protected variable will remain false and provide the ability 
        *           To invoke a session_start() by calling $Class->init();
        */
            if (session_status() === 1){
                $this->Session_Started = true;
                return false; 
            }
            return true;
    }
    public function init(){
        /*
            Invoke a session

        */
        if ($this->Session_Started === false){
            session_start();
            $this->Session_Started = true;
            return true;
        }
        return false;
    }
    public function Status_Session(){
        /*
            Getting the current session status, and return readable information on the current status
        */
        $Return_Switch = false; 
        if (session_status() === 1){
            $Return_Switch = "Session Disabled";
        }elseif (session_status() === 2){
            $Return_Switch = "Session Enabled, but no sessions exist";
        }elseif (session_status() === 3){
            $Return_Switch = "Session Enabled, and Sessions exist";
        }
        return $Return_Switch;
    }
    public function Set ($Key = false, $Value){
        /*
            Set a value within the $_SESSION global
                this function will return true, if a sucessfull addition has been made
                and return false if a problem is enountered
        */
        if ($this->Session_Started === true){
            $_SESSION[$Key] = $Value;
            return true;
        }
        return false;
    }
    public function Get ($Key){
        /*
            Invoking this function will return the current value of the session key, else return false is an error is encountered
        */
        if (isset($_SESSION[$Key])){
            return $_SESSION[$Key];
        }
        return false;
    }
    public function Display(){
        /*
            If session is started, this function will return a readable and formatted array
        */
        if ($this->Session_Started === true){
            return "<pre>".$_SESSION."</pre>";
        }
    }


} // Close class

Now, for example:

$Class = new Session();

if ($Class->Set("Key","Value")){
  echo "Created Value/key within the Session array, and returned true";
}

I understand that people like to validate a successful/fail boolean based on true and false, so I have included the correct return, but my overall question is just to confirm which I believe is correct.. Which is regarding the returns... Is it a perfectly acceptable thing to do, over having no returns? and within best preference?

Answer 1

As for your question, at least Get should not return false if the value is not set, now the caller will not know whether the actual value is false or whether the value is not set.

Other than that,

• I would suggest you use PHP_SESSION_DISABLED , PHP_SESSION_NONE and PHP_SESSION_ACTIVE instead of 1, 2 and 3

• To have Status_Session call session_status is confusing, maybe call it session_status_string ? Also, this should not return false, but maybe "Session status could not be determined"

• I dont understand this : if (session_status() === 1){ $this->Session_Started = true; if session_status() == 1, then sessions are disabled, why would you set Session_Started to true ?

• session_start() will return a boolean that indicates whether the session actually started, you should check that boolean instead of assuming success with $this->Session_Started = true;

• session_start() also can resume a session, I am not a PHP master, but it seems to me you could replace init() with

public function init(){
    /* Start or Resume a session */
    $this->Session_Started = session_start();
    return $this->Session_Started;
}

• Finally, if you are adamant about checking whether the session is started or not in set, would it not make sense to start the session for the caller, so that your code works auto-magically ?

public function Set ($Key = false, $Value){
  if (!isset($_SESSION)) { 
    init(); 
  }
  $_SESSION[$Key] = $Value;
}

Also, as mini-tech commented, if you expect a session, and it is not there, then you might want to throw an exception instead of returning false.

Answer 2

This class isn't really usefull. What if you suddenly decide to not use $_SESSION but a db connection for your session storage? You would then have a lot of duplicate code.

Also the naming of your methods are horrible. Now I would have to read the entire doc/method code to know what really happens. a method nams as getSessionStatus() tells me I'm getting the session status. But Session_Status() doesn't really say a lot, is it for getting? setting? does it toggle the session_status? Oh, it returns a string e.d. representation data. Nah thx, I'll just stick to the session_status() php function.

Then init(). Isn't this simply another name for a __construct()? obviously session's have to be enabled for the class to function. If it isn't enabled on __construct() simply throw an exception and remove that weird init() function and all those if( session_Satrted )

But to answer your original question:returning a boolean in a __construct() is stupid... If you need something that isn't present, throw an Excpetion. If you don't throw an exception then you should work perfectly. Then all methods should have NO SIDE-EFFECTS. If I get a value from the session, I'm only getting a value. I'm not telling you to start a session... So don't do that.

Think DRY SOLID and write your code using a better 'standard': http://www.php-fig.org/psr/1/