1
\$\begingroup\$

I'm trying to develop a better function and curious what others would suggest I do to it so that its not so robust and still accomplish what I need.

A couple of things I need to account for is when I have a last 5 messages as well as getting just the regular number of messages.

/**
 * Gets all or last $x number of personal messages of the specified user
 *
 * @param integer $user_id   User ID of the user specified
 * @param integer $limit   Limit of how many messages to retrieve
 * @param integer $timezone   The timezone offset of the user's geo location
 * @param string  $box   Loading either the inbox or outbox of messages
 * @param string  $date_format   The format for which the date needs to be ouputted
 * @param array   $display  Tells what kind of messages to retrieve (important or specific datetime or datetime range)
 * @return object/NULL
 */
public function get_personal_messages($user_id, $limit = NULL, $timezone, $box, $date_format, $display = NULL)
{
    $this->db->select('personal_messages.message_id');
    $this->db->select('personal_messages.subject');
    $this->db->select('personal_messages.datetime_sent');
    $this->db->select('personal_messages.attachments');
    $this->db->select('personal_messages.priority');
    $this->db->select('personal_messages.message_content');
    if ($box == 'inbox')
    {
        $this->db->select('personal_messages.to_read AS message_read');
    }
    else
    {
        $this->db->select('personal_messages.from_read AS message_read');
    }
    if ($box == 'inbox')
    {
        $this->db->select('personal_messages.is_inbox_favorite AS is_favorite');
    }
    else
    {
        $this->db->select('personal_messages.is_outbox_favorite AS is_favorite');
    }
    $this->db->select('CONCAT(users.first_name, " ", users.last_name) AS sender_name', FALSE);
    $this->db->select('users.email_address AS sender_email_address');
    $this->db->select('user_profiles.user_avatar AS sender_avatar');
    $this->db->from('personal_messages');
    $this->db->join('users', 'users.user_id = personal_messages.from_user_id');
    $this->db->join('user_profiles', 'users.user_id = user_profiles.user_id');
    if ($display['type'] == 'today')
    {
        $this->db->where('datetime_sent >', $display['values']['start_time']);
        $this->db->where('datetime_sent <', $display['values']['end_time']);
    }
    elseif ($display['type'] == 'this_week')
    {

    }
    elseif ($display['type'] == 'last_week')
    {

    }
    if ($box == 'inbox')
    {
        $this->db->where('personal_messages.to_user_id', $user_id);
    }
    else
    {
        $this->db->where('personal_messages.from_user_id', $user_id);
    }
    if ($limit != NULL)
    {
        if (is_numeric($limit))
        {
            $this->db->limit($limit);
        }
    }
    $query = $this->db->get();
    $personal_messages = array();
    $personal_messages['messages'] = $query->result();
    if (count($personal_messages['messages']) > 0)
    {
        for ($x = 0; $x < count($personal_messages['messages']); $x++)
        {
            $attachments = $personal_messages['messages'][$x]->attachments;
            if ($this->functions_model->null_check($attachments) === FALSE)
            {
                $attachments = json_decode($attachments, TRUE);
                for ($i = 0; $i < count($attachments); $i++)
                {
                    $file_name = $attachments[$i];
                    $attachments[$i] = array();
                    $attachments[$i]['file_name'] = $file_name;
                    if ($this->functions_model->is_file('assets/downloads/'.$file_name, FALSE) === TRUE)
                    {
                        $attachments[$i]['is_file'] = TRUE; 
                        $file_size =  $this->functions_model->bytes_to_size(filesize('assets/downloads/'.$file_name));
                        $attachments[$i]['file_size'] = $file_size;   
                        $attachments[$i]['file_location'] = 'assets/downloads/'.$file_name;                
                    }
                    else
                    {
                        $attachments[$i]['is_file'] = FALSE;    
                    }
                }
                $personal_messages['messages'][$x]->attachments = $attachments;
            }
            $personal_messages['messages'][$x]->datetime_sent = $this->functions_model->actual_time('Y-m-d g:i:s', $timezone, strtotime($personal_messages['messages'][$x]->datetime_sent));
            $personal_messages['messages'][$x]->datetime_sent = date($date_format, strtotime($personal_messages['messages'][$x]->datetime_sent)); 
            $personal_messages['messages'][$x]->datetime_sent = $this->functions_model->time_since(strtotime($personal_messages['messages'][$x]->datetime_sent));
            $avatar = $this->functions_model->site_url().'assets/themes/'.$this->config->item('default_theme').'/images/avatars/avatar.jpg';
            if ($this->functions_model->null_check($personal_messages['messages'][$x]->sender_avatar) === FALSE)
            {
                if ($this->functions_model->is_file('assets/themes/supr/images/avatars/'.$personal_messages['messages'][$x]->sender_avatar, FALSE) === TRUE)
                {
                    $avatar = $this->functions_model->site_url().'assets/themes/'.$this->config->item('default_theme').'/images/avatars/'.$personal_messages['messages'][$x]->sender_avatar;
                }
            }
            $personal_messages['messages'][$x]->sender_avatar = $avatar; 
        }
        $personal_messages['total_unread_messages'] = $this->get_users_unread_messages($user_id);
    }
    return $personal_messages;
}
\$\endgroup\$
1
  • \$\begingroup\$ Any ideas for me? \$\endgroup\$ Nov 3, 2012 at 16:14

1 Answer 1

1
\$\begingroup\$

You have a lot of if ($box == 'inbox') statements. Why not bunch them together into one if statement?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.