CODEWITHSUNDEEP
php
Jonathan Bender
Jonathan Bender

Reputation: 1909

PHP switch returning inconsistent results

I have the following switch statement, which should be returning an array of messages sent to/from a user:

<?php
// Load libs, verify user
require_once '../bootstrap.php';

switch ($_GET['folder']) {
  case 'inbox':
    $messages = Message::sentToUser($userId);
    break;

  case 'sent':
    $messages = Message::sentByUser($userId);
    break;

  default:
    $messages = Message::sentToUser($userId);
    break;
}

$json = json_encode($messages);
echo $json;

Messages.php

  public static function sentToUser($user_id)
  {
    $messages = DB::table('messages')
                  ->order_by('id', 'DESC')
                  ->where('sent_to_id', '=', $user_id)
                  ->where('show_to_receiver', '=', 1)
                  ->get();

    return ($messages) ? $messages : array();
  }

Everything works fine if I pass in a folder param:

[
  {
    id: "1"
    title: "Test Message"
    message: ""
    read: "0"
    date_sent: "2014-05-14 15:08:43"
    date_read: null
    sent_from_id: "5"
    sent_to_id: "40"
  }
]

However, in the default case (no folder param given), PHP throws PHP Notice: Undefined index: folder (expected), and $messages is returned as an object, not array:

{
  id: "1"
  title: "Test Message"
  message: ""
  read: "0"
  date_sent: "2014-05-14 15:08:43"
  date_read: null
  sent_from_id: "5"
  sent_to_id: "40"
}

I would have expected $messages to be set to the same thing if $_GET['folder'] is 'inbox' or undefined. Any ideas as to the root of the inconsistency?

Upvotes: 0

Views: 69

Answers (3)

potashin
potashin

Reputation: 44581

It means that $_GET['folder'] is not defined, so you better use isset() to check it.
P.S. : you should always validate user's input (like $_GET and $_POST)

Upvotes: 1

Francisco Presencia
Francisco Presencia

Reputation: 8851

You are not checking whether they are requesting a valid folder or not, which might become important. It's better if you do something along the lines of what @user574632 suggested, but I'd add some more code. Why would $_GET['folder'] be empty? Why would it be a different value? You should account for these two different cases. If the folder can be empty and it means inbox, then say so in your code. This becomes more appropriate then:

// Acknowledge only for the empty folder if it follows your logic, not for everything
$folder = isset($_GET['folder']) ? $_GET['folder'] : 'inbox';

switch ($folder) {
  case 'inbox':
    $messages = Message::sentToUser($userId);
    break;

  case 'sent':
    $messages = Message::sentByUser($userId);
    break;

  // Other code would cause an exception, error, exit or whatever. LOG THAT.
  default:
    throw new InvalidFolderException("User trying to access an invalid folder");
    break;
}

Upvotes: 1

Steve
Steve

Reputation: 20469

$folder = isset($_GET['folder']) ? $_GET['folder'] : '';
switch ($folder) {
...

Upvotes: 4

Related Questions