HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Personal Messsage Model

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
messsagemodelpersonal

Problem

I'm just wondering how my model looks. And by this I mean did I perform the correct tests with the results on each function and use the return at the right points. If you can check all functions. I'm curious to know about if I did the sendMessage function correctly.

```
db->from('usersPersonalMessages AS pm');
$this->db->join('usersPersonalMessagesRecipients AS pmr', 'pm.id = pmr.usersPersonalMessagesID');
$this->db->where('pmr.userID', $userID);
$this->db->where('pmr.messageRead', '0');
$query = $this->db->get();
if ($query->num_rows() > 0)
{
return $query->num_rows();
}
else
{
return 0;
}
}

function getInboxMessagesCount($userID)
{
$this->db->from('usersPersonalMessages AS pm');
$this->db->join('usersPersonalMessagesRecipients AS pmr', 'pm.id = pmr.usersPersonalMessagesID');
$this->db->where('pmr.userID', $userID);
$query = $this->db->get();
if ($query->num_rows() > 0)
{
return $query->num_rows();
}
else
{
return 0;
}
}

function getLast5Messages($userID)
{
$this->db->select('pm.id');
$this->db->select('pm.subject');
$this->db->select('users.firstName');
$this->db->select('users.lastName');
$this->db->select("DATE_FORMAT(pm.dateSent, '%M %D, %Y') AS dateSent", FALSE);
$this->db->select('pmr.messageRead');
$this->db->from('usersPersonalMessages AS pm');
$this->db->join('users', 'users.userID = pm.senderID');
$this->db->join('usersPersonalMessagesRecipients AS pmr', 'pm.id = pmr.usersPersonalMessagesID');
$this->db->where('pmr.userID', $userID);
$this->db->order_by('pm.dateSent', 'desc');
$this->db->limit(5);
$query = $this->db->get();
if ($query->num_rows() > 0)
{
return $query->result();;
}
else
{
return array();
}
}

function getInboxMessages($userID)
{
$this->db->select('pm.id');
$this->db->select('pm.subject');
$this->db->select('users.firstName

Solution

Well taking a brief look and not knowing everything your code is doing in general outside of this model, I would assume its fine. However down at the bottom the last if-else statement you have.

return $query->row();;


which is an extra semi-colon. Remove that, and I'd say your code looks clean and well structured. Give it a run, see if it works.

One thing to keep in mind is coding is a lot of trial and error, and with that there is no one particular way to handle any given set of events you want to create. Meaning I might be able to recreate the same functionality in a far different way then yours. Doesn't mean its right or wrong. At the end of the day as long as what your building runs smooth and fast without error I would think your or anyone in general good to go.

On a different note however. The functions you have parameters being passed to. Maybe this is me being a stickler. But you may want to error check those before setting up and running your query. Error check in this sense to me means make sure they aren't null, empty, make sure they are set, and depending on what you might be wanting to pass such as specifically an integer or maybe a value that resembles a string that is email address format. I only say this cause why bother running the query or even attempting if the criteria doesn't fit the need.

Code Snippets

return $query->row();;

Context

StackExchange Code Review Q#10378, answer score: 2

Revisions (0)

No revisions yet.