[eluser]jedd[/eluser]
Okay, some of this - I'll throw some ideas at you, but someone else may well correct me later.
[quote author="the future darlo manager" date="1236975541"]Right, based on the advice I decided to bundle the queries together in one place.[/quote]
In addition to having queries in the model, you might want to have multiple methods to provide different types of information back. You might want to look at phpdoc - I've started using this lately and it's very useful for inline documenting of functions - parameters, return values, etc. getUserSubmissions returns one submission, right? And that's meant to include a Teacher and a Learner array (as shown), yes?
Your user->row stuff in the controller - shouldn't be there. Your model should be returning an array, and you foreach() your way through that array. This will also make display of those data (in the view, later) much easier.
Code:
function getUserSubmissions($user_id) {
$query1 = $this->db->query("SELECT *,
...
FROM teacher_submissions
WHERE user_id = '$user_id'
LIMIT 1");
Consider:
o SELECTing only the data that you need
o if user_id is the PK, you don't need LIMIT
o if user_id is not the PK, you might want to remove LIMIT so you can test for database consistency
Rather than this:
Code:
if ($query1->num_rows() == 1)
{
foreach ($query1->result_array() as $row)
{
//$submission_data[] = $row;
$submission_data['teacher'] = $row;
}
}
Why not just:
Code:
if ( $query1->num_rows() )
{
$submission_data['teacher'] = $query1->row_array();
}
Instead of just returning data (regardless of whether it exists) with this:
Code:
return $submission_data;
Why not add some meaning to the return value:
Code:
if ($query2->num_rows())
return $submission_data;
else
return FALSE;
Rather than this:
Code:
function show()
{
//Gets the user id from the url
$user_id = $this->uri->segment(3, NULL);
//Finds out if the sort by setting has been issued from the URL
if (!isset($user_id)) {
redirect('users/showall', '');
}
How about this:
Code:
function show ( $user_id = NULL )
{
//Finds out if the sort by setting has been issued from the URL
if (! $user_id) {
redirect('users/showall', '');
}
Instead of this (in the controller):
Code:
if ($data['user']->num_rows() == 1)
{
//Gets the user details for the view
$row = $data['user']->row();
$data['user_id'] = $row->user_id;
$data['email'] = $row->email;
...
As discussed, you want to avoid -> row calls within the controller. The model should be returning an array, so at this point you can just copy the $user array into your $data array, and the $data array will later feed the view, right?
So you'd replace the above with something like:
Code:
$data['user'] = $row_array_result; // (word it better, it's the thing that came back from the model earlier)
In fact you can assign $data['user'] with the output of the model call directly. You still want to test it to make sure that it's got valid data in it (again, in the model you'd return FALSE if you have a problem, or can't find a match etc).
We may be approaching the meat of your question here.
Code:
//Loads the views
$this->load->vars($data);
$this->load->view('admin_header.php');
$this->load->view('show_user.php');
Rather than load->vars($data) you can pass the $data to each view - this is handy if you want to provide less data to each view, but of course not necessary. I think you also need one less
.php in the call. So something like :
Code:
$this->load->view('show_user' , $data);
Within your view, as per the user manual, you don't see $data - you see only the array elements. So for example, the variable $user would exist, and be an array that contains keys such as 'user_id' and 'email', and so on.
Does this make more sense?
