[eluser]zyzzzz[/eluser]
Lets see what everyone elses coding standards look like, how they comment, and how they structure thier controllers. Perhaps we can learn something. Obviously, remove any sensitive information :-) Note: If your controller is absolutely massive, maybe post a different one.
Code.php
Code: <?php if (!defined('BASEPATH')) exit('No direct script access allowed');
class Code extends Controller
{
function __construct()
{
parent::Controller();
}
function index($code_name)
{
$this->load->model('code_model');
$this->load->library('tank_auth');
$this->load->library('form_validation');
// If the code url is not valid, show error message.
$sql = 'SELECT * FROM codes WHERE url_name = ?';
$bindings = array($code_name);
$listing = $this->db->query($sql, $bindings)->row_array();
if (empty($listing))
{
// Show error
return;
}
// If we have feedback, lets post it.
if ($this->tank_auth->is_logged_in() && $this->input->post('feedback'))
{
$this->form_validation->set_rules('feedback', 'Feedback', 'trim|required|xss_clean');
if ($this->form_validation->run())
{
// Insert the data into the feedback table
$sql = 'INSERT INTO feedback(code_id, user_id, feedback) VALUES (?, ?, ?)';
$bindings = array ($listing['id'], $this->tank_auth->get_user_id(), set_value('feedback'));
$this->db->query($sql, $bindings);
}
}
// If we have a rating, lets rate it.
if ($rating = $this->input->post('rating'))
{
if (is_numeric($rating) && $rating >= 1 && $rating <= 5)
{
// Get the number of votes that this IP address has on this code.
$sql = 'SELECT * FROM ratings WHERE INET_NTOA(ip) = ? AND code_id = ?';
$bindings = array ($_SERVER['REMOTE_ADDR'], $listing['id']);
$query = $this->db->query($sql, $bindings);
// We should only let them vote if they haven't already voted.
if ($query->num_rows() == 0)
{
$sql = 'INSERT INTO ratings (code_id, ip, rating) VALUES (?, ?, ?)';
$bindings = array ($listing['id'], $_SERVER['REMOTE_ADDR'], $rating);
$this->db->query($sql, $bindings);
}
}
}
// Add one to the number of views that this code has.
$sql = "UPDATE codes SET views = views + 1 WHERE id = {$listing['id']}";
$this->db->query($sql);
// Retrieve language, category, tag and user arrays and add them to the listing.
$sql = "SELECT * FROM languages WHERE id = {$listing['language_id']}";
$listing['language'] = $this->db->query($sql)->row_array();
unset($listing['language_id']);
$sql = "SELECT * FROM categories WHERE id = {$listing['category_id']}";
$listing['category'] = $this->db->query($sql)->row_array();
unset($listing['language_id']);
$sql = "SELECT username FROM users WHERE id = {$listing['user_id']}";
$listing['user'] = $this->db->query($sql)->row_array();
unset($listing['user_id']);
// Retrieve rating average and rating number and add it to listing
$sql = "SELECT COUNT(rating) as rating_count, SUM(rating) / COUNT(rating) as average FROM ratings WHERE code_id = {$listing['id']}";
$listing['rating'] = $this->db->query($sql)->row_array();
// Retrieve the tags that this listing has
$listing['tags'] = $this->code_model->get_tags($listing['id']);
//echo '<pre>', print_r($listing, true), '</pre>';
//return;
// Render the partial view.
$partial_data = array ('listing' => $listing);
// If the user is logged in, lets add it to the partial data
if ($this->tank_auth->is_logged_in())
{
$sql = 'SELECT * FROM users WHERE id = ?';
$bindings = array($this->tank_auth->get_user_id());
$partial_data['user'] = $this->db->query($sql, $bindings)->row_array();
}
// Get a list of feedback and add it to the partial data
$sql = "SELECT feedback.*, users.username
FROM feedback
INNER JOIN users
ON feedback.user_id = users.id
WHERE feedback.code_id = {$listing['id']}
ORDER BY feedback.time_posted ASC";
$partial_data['feedback'] = $this->db->query($sql)->result_array();
$partial_rendered = $this->load->view('pages/partial/code/index', $partial_data, TRUE);
// Render the layout
$layout_data = array(
'content' => $partial_rendered
);
$this->load->view('pages/layout', $layout_data);
}
function _remap($code_name)
{
$this->index($code_name);
}
}
[eluser]überfuzz[/eluser]
I'm not exactly 100% on what every part of this controller does, but after a glance it seems that you could do with a MY-controller. Make a MY_controller and extend your controllers with it. This way you'll have all things going out to every page in one place. With the my-controller in place let your controllers contain only whats unique for the controller.
Catfish?
[eluser]Boris Strahija[/eluser]
I think it would be better if you would put your SQL statements into a model. There's a reason why CI is MVC 
[eluser]brianw1975[/eluser]
[quote author="Boris Strahija" date="1251656397"]I think it would be better if you would put your SQL statements into a model. There's a reason why CI is MVC [/quote]
++Amen to that...
[eluser]Boris Strahija[/eluser]
Here's a controller from one of my first projects in CI, and generally MVC. It's very far from perfect, but I think it's ok for a start 
It's just the start of the controlle because of the charachter limit.
Code: <?php if (!defined('BASEPATH')) exit('No direct script access allowed');
/**
* Galleries Controller
*
* @author Boris Strahija <[email protected]>
* @copyright Copyright (c) 2009, Boris Strahija, Creo
* @version 0.1
* @since 2009-04-11
*/
class Galleries extends Controller {
var $section = "galleries"; //marking the navi
var $user_level = 0;
/**
* The constructor
*
*/
function __construct()
{
// Call the Controller constructor
parent::Controller();
//restricted access
$this->auth->restrict();
//load required models and libraries
$this->load->model('mdl_galleries');
//profiler
show_profiler($this->section);
//get user level
$this->user_level = $this->auth->get_user_level($this->session->userdata('user_id'));
} //end __contruct()
/**
* Index method
*
*/
function index()
{
$this->overview();
} //end index()
/**
* List of all galleries
*
*/
function overview()
{
if ($this->user_level < 10) redirect('backend/galleries/exibitions');
//set data to pass
$data = array();
//sections
$data['section'] = $this->section;
$data['section2'] = 'overview';
//page title
$data['title'] = $this->lang->line('core_galleries').' / '.$this->lang->line('core_overview');
//headline and extra button
$data['h2'] = $this->lang->line('core_gallery_overview'); //the main headline
$data['hbutton'] = '<span class="action">| '.anchor('backend/galleries/gallery', $this->lang->line('core_new_gallery'), array('class'=>'add')).'</span>';
//content view
$data['content_view'] = 'galleries/grid';
//action URL's (form and delete)
$form_url = base_url() . 'backend/galleries/gallery/';
$delete_url = base_url() . 'backend/galleries/gallery/delete/';
//get all records
$data['list'] = $this->mdl_galleries->get(array('category_id'=>'1'));
//build grid
if ($data['list']) :
//if only one result is returned create array
$data['list'] = to_array($data['list']);
//configure the grid
$grid_cfg = array(
'cols' => array(
array($this->lang->line('label_name'), 'name', NULL, NULL, $form_url.'{id}', NULL)
,array($this->lang->line('label_updated'), 'updated', 140, NULL, NULL, 'unix_to_human')
,array($this->lang->line('label_actions'), '{actions}', 60, 'edit', NULL, NULL)
)
,'actions' => array(
array($this->lang->line('label_edit'), $form_url.'{id}', 'edit')
,array($this->lang->line('label_delete'), $delete_url.'{id}', 'delete')
)
,'sort' => array('0,0', '1,0')
,'pager' => TRUE
,'class' => 'wide'
);
//create grid object
$grid = new grid($data['list'], $grid_cfg);
//get grid code
$data['grid'] = $grid->get(FALSE);
endif;
//load the view
$this->load->view('backend/core', $data);
} //end overview()
/**
* List of all exibitions
*
*/
function exibitions()
{
//set data to pass
$data = array();
//sections
$data['section'] = 'exibitions';
$data['section2'] = 'overview';
//page title
$data['title'] = 'Exibitions / '.$this->lang->line('core_overview');
//headline and extra button
$data['h2'] = 'Exibition overview'; //the main headline
$data['hbutton'] = '<span class="action">| '.anchor('backend/galleries/exibition', 'New exibition', array('class'=>'add')).'</span>';
//content view
$data['content_view'] = 'galleries/grid';
//action URL's (form and delete)
$form_url = base_url() . 'backend/galleries/gallery/';
$delete_url = base_url() . 'backend/galleries/gallery/delete/';
//get all records
$data['list'] = $this->mdl_galleries->get(array('category_id'=>'8'));
//build grid
if ($data['list']) :
//if only one result is returned create array
$data['list'] = to_array($data['list']);
[eluser]zyzzzz[/eluser]
[quote author="Boris Strahija" date="1251656397"]I think it would be better if you would put your SQL statements into a model. There's a reason why CI is MVC [/quote]
I originally did have them all in model files however I found I was writing methods to select, update, insert and delete things from the database, which to me seems counter-intuitive and a waste of time.
For example I had a method called get() which would take an array of criteria (a key which was the field an value with was the value) and get the content of that record (or records) and return it. But in reality it was just more work than its worth, when I could write the SQL statement in my controller and get just what I need, when I needed it.
I still have some things in my models though, for example I have two tables joined by a pivot table, and one of the methods get the things associated with a record in the other table via pivot table and sorts them.
But its far easier to just leave the select/insert/update/delete functions to the controller.
To me, seperating the view from the logic is absolutely important, but seperating database functions from the controller is only done if there is a reason.
[eluser]t'mo[/eluser]
Here's the code behind something I threw together a few weeks ago: http://didyoucry.com/clshop (yes, I need to get this it's own domain name instead of piggy-backing off another old project of mine).
It's more-or-less a tool that automated the way I look for stuff to buy on craigslist: 1) search for X in my town; if not found in my town, 2) search for X in the next town over by typing in the next town's name in the url.
Code: <?php
class Site extends Controller
{
function index() {
$this->load->view('pageTemplate',
array('content' => $this->load->view('queryForm', null, true)));
}
function search() {
$query = $this->input->post('query');
$cities = array();
if (!empty($query)) {
$this->load->model('Lookup');
$zip = $this->input->post('zip');
if (!empty($zip)) {
$cities = $this->Lookup->citiesNearZip($zip, $this->input->post('radius'));
}
else {
$cities = $this->Lookup->cities();
}
}
$data = array(
'cities' => $cities,
'query' => $query,
'hasPic' => $this->input->post('hasPic'));
$cityList = $this->load->view('cityList', $data, true);
$this->load->view('pageTemplate', array('content' => $cityList));
}
}
Some notes on my "style":
- database calls do not belong in the Controller
- HTML does not belong in the Controller
- deploy application code up and out webroot; the "if (! defined('BASEPATH'))..." stuff just adds visual clutter
- I'm picky about placement of curly braces, blank lines, spaces, etc.
- too many comments, especially a play-by-play sportscaster-style callout of every f@!#$# little thing really annoys me; the code ought to be clear enough to speak for itself
One critique I have of my own code here is that I'm not validating the input, which I should; look for that in v0.5, I guess. :-) (I should also put in a copyright notice, though I have that in the HTML; I'm lazy) And let me know your critiques of this code too.
Update: mini-rant about comments, copyright notices
[eluser]Boris Strahija[/eluser]
[quote author="James Brauman" date="1251705590"][quote author="Boris Strahija" date="1251656397"]I think it would be better if you would put your SQL statements into a model. There's a reason why CI is MVC [/quote]
I originally did have them all in model files however I found I was writing methods to select, update, insert and delete things from the database, which to me seems counter-intuitive and a waste of time.
For example I had a method called get() which would take an array of criteria (a key which was the field an value with was the value) and get the content of that record (or records) and return it. But in reality it was just more work than its worth, when I could write the SQL statement in my controller and get just what I need, when I needed it.
I still have some things in my models though, for example I have two tables joined by a pivot table, and one of the methods get the things associated with a record in the other table via pivot table and sorts them.
But its far easier to just leave the select/insert/update/delete functions to the controller.
To me, seperating the view from the logic is absolutely important, but seperating database functions from the controller is only done if there is a reason.[/quote]
I understand that it's easier for you, and I ceartainly won't preach MVC to you It just seems much cleaner code to me when I separate everything in the MVC way, and taht means for me that all data stuff should be in a model. I think it's also important when more than 1 person are working on the project. It can quite easy become a mess if you don't follow the basic rules.
[eluser]n0xie[/eluser]
I think a lot of people have a lot of code that can be abstracted away...
For example, a generic controller in our company looks like this:
Code: class Home extends Frontend_Controller {
function Home()
{
parent::Frontend_Controller();
}
function index()
{
$this->load->model('routing_model','routing');
$route_id = $this->routing->get_route_id('home', $this->lang->lang());
$this->load->model('cms_model','cms');
$data = $this->cms->get_properties($route_id);
$this->set_template($data->template_id);
$this->render($data);
}
}
[eluser]narendran[/eluser]
hi,
I'm doing Cart system like products,category,order etc.for this i have to write code for
admin area to edit,delete,add,giving authorization to customers to login or not.
frnds, I'm Beginner of CI.so help me how to do this by CI.give me reference sites.sample codes also.
hope i will get a Good response from u.
thanks,
Narendran
|
