[eluser]iConTM[/eluser]
I'm working on a backend where each category (members, articles, ...) access a controller of the following structure:
Code: <?php
class Members extends Controller {
public function Members ()
{
parent::Controller();
#load assets
$this->load->database();
$this->load->helper('form');
$this->load->helper('common');
}
/* lists all records */
public function index ()
{
#remove form inputdata
$this->load->library('session');
$this->session->unset_userdata('form');
$this->load->model('member_model');
$data['rows'] = $this->member_model->get_list();
$this->load->view('members_list', $data);
}
/* shows one single record */
public function show ($id)
{
$id or show_404();
# get record from database
$this->load->model('member_model');
$row = $this->member_model->get($id);
# set data
$data['id'] = $row->id;
$data['username'] = $row->username;
// ...
$this->load->view('members_detail', $data);
}
/* shows a form to add a record */
public function add ()
{
#get inputdata from session
$this->load->library('session');
$data = $this->session->userdata('form');
if (!$data)
{
# set inputdata
$data['output'] = '';
$data['id'] = '';
$data['username'] = '';
// ...
}
$data['country_options'] = countries();
$this->load->view('members_form', $data);
}
/* shows a form to edit a record */
public function edit ($id=NULL)
{
$id or show_404();
#get inputdata from session
$this->load->library('session');
$data = $this->session->userdata('form');
if (!$data)
{
# get record from database
$this->load->model('member_model');
$row = $this->member_model->get($id);
# set inputdata
$data['output'] = '';
$data['id'] = $row->id;
$data['username'] = $row->username;
// ...
}
$data['country_options'] = countries();
$this->load->view('members_form', $data);
}
/* save a record to the database */
public function save ()
{
# referrer
$this->load->library('user_agent');
if ( !$this->agent->is_referral() )
die('Invalid referrer.');
# validate
$rules = array();
$rules['email'] = 'required|valid_email';
$this->load->library('validation');
$this->validation->set_rules($rules);
if ($this->validation->run() == FALSE)
{
# invalid - return
$data['output'] = $this->validation->error_string;
$data = array_merge($_POST, $data);
$this->load->library('session');
$this->session->set_userdata('form', $data);
redirect($this->agent->referrer());
}
else
{
# valid
# parse form
$member = new stdClass();
$member->id = $this->input->post('id');
$member->username = $this->input->post('username');
// ...
# save
$this->load->model('member_model');
if ($this->member_model->save($member))
{
// record successfully saved.
redirect('members');
}
else
{
// Could not save record. Try again later
// back to form
}
}
}
/* deletes a record from the database */
public function delete ($id=NULL)
{
$id or show_404();
$this->load->model('member_model');
if ( $this->member_model->delete($id) )
{
// record successfully deleted.
}
else
{
// Could not delete record. Try again later
}
redirect('members');
}
}
/* End of file members.php */
/* Location: ./application/controllers/members.php */
I feel I'm writing to much code inside a controller.
Any tips on good practice?
tx
[eluser]iConTM[/eluser]
Here is my model class
Code: <?php
class Member_model extends Model
{
public function Member_model ()
{
parent::Model();
}
public function get_list ()
{
$query = $this->db->get('ms_members');
return $query->result();
}
public function get ($id)
{
$this->db->where('id', $id);
$query = $this->db->get('ms_members');
return $query->row();
}
public function save (&$vo)
{
if ( isset($vo->id) && $vo->id && $vo->id != '' )
return $this->_update($vo);
return $this->_insert($vo);
}
protected function _insert (&$vo)
{
if ( isset($vo->id) )
unset($vo->id);
$this->db->insert('ms_members', $vo);
$success = $this->db->query($sql);
$vo->id = ($success) ? $this->db->insert_id() : false;
return $success;
}
protected function _update (&$vo)
{
$id = $vo->id;
unset($vo->id);
$this->db->where('id', $id);
$success = $this->db->update('ms_members', $vo);
$vo->id = $id;
return $success;
}
}
[eluser]WanWizard[/eluser]
Looks fine to me. I've got controllers with a lot more code... :-)
[eluser]iConTM[/eluser]
Maybe I just want to be too precise 
tx for the feedback 
[eluser]bretticus[/eluser]
Quote:I feel I’m writing to much code inside a controller.
Really, your controller seems pretty lean. In fact, you're abstracting form validation out of the model, which is fine. It just depends on how modular you want your models to be. For example, do you want a black box that does nothing but take parameters and handle database transactions? Do you want your models to validate the form data in addition to CRUD operations? Your choice really. Keep in mind that controllers, in addition to being your application's access points, coordinate data FROM the views (forms, etc.) to your models besides retrieving data to push TO the view. Sure, your code is more modular if your controller knows very little about the data other than being the medium for transport. However, I see no problem with your coding style here (in fact, it's very good in my opinion.)
As for your more "wordy" code, you are basically looking for session data to send to your view pages and pulling data from the database. I suppose that you could handle the session look-ups from your model(s) and have "deeper" model methods for the actual CRUD operations. Your "higher level" model methods would then ultimately consult the "deeper" ones. That way, you can abstract the repetitive nature of the session data look-up and maintain CRUD methods just in case. That is probably the only thing I could see you improving on after a few quick glances.
I'm sure others will give their input and I'll be very interested to see it.
[eluser]iConTM[/eluser]
For now I have like 20 controllers with the same structure as above :p
Maybe it's better for me to inherit the CRUD structure from a main controller (MY_Controller)
I'll take a look at my models to make them more modular.
thanks for the comments bretticus.
greetings
[eluser]tkyy[/eluser]
you don't need help, i really like your coding structure. you are keeping the database calls into the models and you are loading your assets in the controller. you are doing great so far!
[eluser]tkyy[/eluser]
btw here i will give you a sloppy example of one of my controllers
Code: <?php
/**
* friends controller
*
* this controller takes care of the friends page
* and related pages. it also incorperates the openinviter tool
* as well as search and browse functions into the site, that
* are called off the friends page
*
* also, in the header when you search for someone by name, it is
* handled by this controller as well
*/
class friends extends controller
{
/* userdata classvar */
var $userdata;
/* constructor */
function friends()
{
parent::controller();
/**
* since this entire controller is internal, we can specifiy the
* internal() function in the constructor function instead of calling
* it in each function inside the controller
*/
internal();
/**
* find the userdata of the user using the helper function
*/
$this->userdata = userdata();
/* show activity for this user */
activity($this->userdata['id']);
}
/**
* just shows the friends view, and passes the userdata/friends down. no logic
* here really
*/
function index()
{
/* pass the userdata */
$data['userdata'] = $this->userdata;
$data['friend_requests_count'] = $this->db->select('*')
->from('friends')
->where('user_id_to',$this->userdata['id'])
->where('time_accepted','0')
->get()
->num_rows();
$this->load->view('internal_friends',$data);
}
/**
* this is the function that is called from the profile page to
* request a user as a friend. it shows a popup view and asks the user
* to confirm the add, and also allows for a small greeting message
* of 50 characters.
*/
function request($user_id='')
{
/* load the users model */
$this->load->model('users');
/* validate the user_id numerically first */
if(!is_numeric($user_id) || $user_id=='')
{
/* show a 404 if it was invalid */
show_404();
}
/* grab the user from the table */
$user_to = $this->users->grab($user_id);
/* validate again, make sure the data returned was an array */
if(is_array($user_to))
{
/**
* this was a valid person, lets show the confirmation message now, allowing
* them to enter a small personalized message along with the request
*/
/* pass the profile data for the user we are sending to */
$data['user'] = $user_to;
/* show the viewfile */
$this->load->view('internal_popup_friend_request_confirm',$data);
}else{
/* this was an invalid user, show a 404 */
show_404();
}
}
/**
* this is the confirmation of the above function, it takes post input
* and does the actual friend request
*/
function request_send($user_id='')
{
/* load the users and friends models */
$this->load->model('users');
$this->load->model('friends_m');
/* validate the user_id numerically first */
if(!is_numeric($user_id) || $user_id=='')
{
/* show a 404 if it was invalid */
show_404();
}
/* grab the user from the table */
$user_to = $this->users->grab($user_id);
/* validate again, make sure the data returned was an array */
if(is_array($user_to))
{
/* attempt to add the user into the database */
$result = $this->friends_m->request( $user_id,
$this->userdata['id'],
$this->input->post('message') );
/**
* if the result of the query was an array, it means that there
* was already a relationship between the users
*
* we can check the status of the request by checking time_accepted.
* if the value isn't 0, then they are friends already.
*/
if(is_array($result))
{
$check_1 = $this->db->select('*')->from('friends')->where('user_id_to',$this->userdata['id'])->where('user_id_from',$user_id)->limit(1)->get()->num_rows();
$check_2 = $this->db->select('*')->from('friends')->where('user_id_to',$user_id)->where('user_id_from',$this->userdata['id'])->limit(1)->get()->num_rows();
if($check_1==1 || $check_2==1)
{
/**
* these users are not friends, but they have a pending friend
* request already, set the flash data indicating that
*/
flash('There is already a pending friendship between you and '.ucwords($user_to['first_name'].' '.$user_to['last_name']),'red');
}else{
/**
* these users are already friends, set the flash data indicating that
*/
flash('You are already friends with '.ucwords($user_to['first_name'].' '.$user_to['last_name']),'red');
}
}elseif($result==1)
{
/**
* if the result was one, then a request was sent to this user,
* set the flash data indicating that
*/
meh, it cut my code off!
[eluser]tkyy[/eluser]
anyway that should give you an alright idea of how controllers work (for me anyway, everyone has their own preferences). i'm leaning towards refining my syntax now (i no longer heavily comment my work and i also have switched to not putting linebreaks after function names and before { carrots).
just keep coding, as long as your code works there is no reason to change anything unless you foresee a problem with scalability or readability in the future.
[eluser]iConTM[/eluser]
Thanks for the post tkyy.
Interesting coding. I'll take a look at it 
greetings
|
