views:

85

answers:

2

I have been using my own method for years, but I figured maybe its not the best way to go about it.

Essentially when I want to throw an error to a user, or display confirmation of a successful action, I do the following:

if($something == "condition") {

   $_SESSION["message"] = "Your passwords didnt match! Make sure they are the same in both fields!";
   $_SESSION["message_type"] = 1;
   header("Location:register.php");
   exit();

}

then I have a function like

function show_message() {
   global $_SESSION;

   if (isset($_SESSION["message"])) { 
      echo "<div class='site_message type_" . $_SESSION["message_type"] . "'>" . $_SESSION["message"] . "</div>"; 
      unset($_SESSION["message"]); 
      unset($_SESSION["message_type"]); 
   }
}

and I put show_message(); on top of every page to display possible errors that might be throw to this page.

What are the possible problems with this?

+2  A: 

I see nothing wrong with this approach. You'll find this technique under different names in quite a number of frameworks, for instance FlashMessenger in Zend Framework. Usually the Session is wrapped in an object instead of the regular Session array and with a ViewHelper instead of a function.

To make sure you don't have any typos in the Session keys when assigning the message, you could wrap the assigning code into a function as well, e.g.

function set_message($text, $type)
{
    $_SESSION['message'] = array(
        'text' => $text,
        'type' => $type
    );
}

You could improve it by having the function return the string instead of echoing it and personally I'd use sprintf to format the output. Makes the code somewhat more readable imho, e.g.

return sprintf('<div id="message-box" class="type-%s">%s</div>',
                $_SESSION["message"]["text"], 
                $_SESSION["message"]["type"]);

Like @Gumbo pointed out, the function might not work when Sessions do not work, but that will likely impose a number of other problems for the entire application then, so I wouldn't exactly bother about this particular piece of code then.

Minor thing: $_SESSION is a superglobal, so you do not have to use the global keyword.

Gordon
What do you mean by "badly formatted"?
Yegor
@Yegor it's fine now. The `unset`s was on the same line as the echo.
Gordon
+1  A: 

Honestly, I would do this not with Sessions, but with the URL.

Instead of setting a message, make a class that contains 'System Messages'

 class SystemMessages{
    protected $messages = array(
          0 => "Some Error Message");

    public function getMessage($id)
    {
        return $this->messages[$id];
    }
 }

Then, on your register.php, check for a URL parameter:

$messageObject = new SystemMessages;
if(!empty($_GET['message']))
{
    $message_id = intval($_GET['message']); // Clean User Input
    $message = $messageObject->getMessage($message_id);
    // handle message
}

If you are ONLY showing a message, then it really isn't 'sensitive' data, and therefore it should be perfectly fine to put it in the URL.

Then you just

 header('Location: register.php?message=0');

Works with/without cookies, and you it much more centralized. If you wanted to change the wording of a message, all of the messages are in the same place.

Just a thought....

Chacha102
It's common practice to use sessions for messages.
rick