CodeIgniter Forums
How can I get rid of this logic from the view? - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Archived Discussions (https://forum.codeigniter.com/forumdisplay.php?fid=20)
+--- Forum: Archived Development & Programming (https://forum.codeigniter.com/forumdisplay.php?fid=23)
+--- Thread: How can I get rid of this logic from the view? (/showthread.php?tid=56882)



How can I get rid of this logic from the view? - El Forum - 01-27-2013

[eluser]behnampmdg3[/eluser]
Hello;

In this simple example I retrieve data from the database and simply show it on screen:

Controller:
Code:
$this->load->model('celebrities_model');
$results = $this->celebrities_model->list_celebrities(5);
$data['celebrities'] = $results->result_object;
Model
Code:
public function list_celebrities($limit=NULL)
{
  $this->db->select('...');
  $this->db->from('sincity_celebrities');
  $this->db->join('sincity_events_celebrities', 'sincity_events_celebrities.celebrity_id = sincity_celebrities.id', 'LEFT OUTER');
  $this->db->where('type !=', 'DJ');
  $this->db->group_by("sincity_celebrities.id");
  $this->db->order_by("sincity_events_celebrities.id DESC");
  $this->db->limit($limit);
  $query = $this->db->get();
  foreach($query->result() as $row)
   {
     $this->result[]=array('id'=>$row->id,'name'=>$row->name, 'facebook'=>$row->facebook, 'photo'=>$row->photo, 'twitter'=>$row->twitter, 'website'=>$row->website, 'type'=>$row->type, 'EID'=>$row->EID);
   }
   return $query;
}
View
Code:
foreach($celebrities as $celebrity){ echo $celebrity->photo;}
Now where wuld be the best place to put this logic? It is currently in view and it is hurting my feelings
Code:
if($celebrity->EID!=NULL)
     {
          echo $celebrity->EID;
     }
Thanks


How can I get rid of this logic from the view? - El Forum - 01-27-2013

[eluser]Aken[/eluser]
Why are you reassigning all of your columns in the model? And why are you doing it directly to the result property? That's so weird and unnecessary.

Code:
// Your other $this->db stuff...

$query = $this->db->get();

return $query->result_array();

And this is basically the same answer as last time. Do the logic in your controller before passing it to your view.


How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]behnampmdg3[/eluser]
[quote author="Aken" date="1359343394"]Why are you reassigning all of your columns in the model? And why are you doing it directly to the result property? That's so weird and unnecessary.

Code:
// Your other $this->db stuff...

$query = $this->db->get();

return $query->result_array();

And this is basically the same answer as last time. Do the logic in your controller before passing it to your view.[/quote]Ok Aken, thanks for the help.

How about this:
Model
Code:
<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');
class Test_model extends CI_Model
{
  public $event_list=array();
  public function list_results()
   {
    $this->db->limit('5');
    $query = $this->db->get('sincity_events');
    return $query->result_array();
   }
}
Controller
Code:
<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class Test extends CI_Controller {

public function index()
  {
   $this->load->model('test_model');
   $data['results'] = $this->test_model->list_results();
   $this->load->vars($data);
   $this->view_things();

  }

public function view_things()
   {
    $this->load->view('test_view');
   }
}?>
View
Code:
<ul>
&lt;?php foreach ($results as $item):?&gt;
<li>&lt;?php echo $item['title'];?&gt;</li>
&lt;?php endforeach;?&gt;
</ul>



How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]Aken[/eluser]
I'm glad someone doesn't mind my often-rash attitude, lol.


How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]behnampmdg3[/eluser]
[quote author="Aken" date="1359684374"]I'm glad someone doesn't mind my often-rash attitude, lol.[/quote]Well, "ego" = failure.

Would you please check the edit above (reply #2) I just edited Smile . Thanks dude



How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]Aken[/eluser]
If you're going to separate the logic that generates the views into a view_things() method, you should make it private or put an underscore on the front, otherwise it can be called through the browser.

Code:
private function view_things() {}
// OR
function _view_things() {}

Otherwise that's a perfect starting point. Basically, if you find yourself doing logic, like comparisons or checking for empties, in your view, you can do it in your controller instead if you want to keep the code clean. Doing some logic is okay, it can just get really complicated to read if you have a lot. That's where template scripts like Smarty and Twig come in handy. Or, if you're just echoing variables, you can use CI's Parser library instead of doing a ton of opening/closing PHP tags.


How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]behnampmdg3[/eluser]
Ok thank you. About logics, comparison and other things I have a question. I am sure you are gonna see my code and have tones of comments.

I have a page where client uploads photos for a page. I used to have all the logic in controller but then I realised controllers are not supposed to carrey too much logic, and it may not be a bad idea to move the functionalities elsewhere. So I did I thought I make my own controller and place it in libraries. You probably suggest a better place than libraries folder for my own scripts.
Anyways this is the controller and after that my own script whic is located inlibraries.

So in general:

1 - What would you change?
2 - Is this well written code?

Thank you Aken
Code:
public function edit($page)
  {
   $this->load->model('page_content_model');
   if($this->page_content_model->check_page($page))
    {
      $results = $this->page_content_model->load_page_components($page);
     foreach($results as $val)
      {
       foreach($val as $column=>$value)
        {
         $data['page_data'][] = array($column=>$value);
        }
      }
     $this->load->library('validations/'.$page,'',$page);
     $this->$page->validate();
     $this->load->vars($data);
    
     $this->load->view('header_view');
     $this->load->view($page);
     $this->load->view('footer_view');  
    }
   else
    {
     redirect(base_url().'not_found', 'location', 301);
    }
  }
And this is located in libraries
Code:
&lt;?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class Sincity_content_booths  
  {
   public function validate()
    {
     $CI =& get_instance();
     foreach(array('main_title', 'paragraph', 'col_one_title', 'col_one_text', 'col_two_title','col_two_text', 'col_three_title', 'col_three_text', 'second_title', 'photo_one',  'photo_two', 'photo_three') as $key)
      {
       $data[$key] = $CI->input->post($key);
      }
     $CI->load->vars($data);
     if($CI->input->post('form_submit'))
      {
       ////Photo Management
       $CI->posted_photo = array('photo_one', 'photo_two','photo_three');
       foreach($CI->posted_photo as $photo)
          {
          
           if(strlen($_FILES[$photo]['name'])>0)
            {
             $config['upload_path'] = SITE_PATH."images/booths/";
             $CI->photo_name = time().rand(1,10000000);
             $config['file_name'] = $CI->photo_name.".jpg";
             $config['allowed_types'] = 'gif|jpg|png';
             $field_name = $photo;
             $CI->load->library('upload');
             $CI->upload->initialize($config);
             if ( ! $CI->upload->do_upload($field_name))
              {
               $error[] = array('error' => $CI->upload->display_errors());
              }
             $config_resize['source_image'] = $config['upload_path'].$CI->photo_name.".jpg";
             $config_resize['overwrite'] = TRUE;
             $config_resize['create_thumb'] = FALSE;
             $config_resize['height'] = 132;
             $config_resize['width'] = 326;
             $config_resize['maintain_ratio'] = TRUE;
            
             $CI->load->library('image_lib', $config_resize);
             $CI->image_lib->clear();
             $CI->image_lib->initialize($config_resize);
             if ( ! $CI->image_lib->resize())
              {
               $CI->image_lib->display_errors();
              }
             else
              {
               $data[$photo]=$CI->photo_name;
              }
            }
          }
        
       //// End photo management
       $CI->form_validation->set_rules('main_title', 'Title', 'required');
       $CI->form_validation->set_rules('paragraph', 'Page Text', 'required');
       if($CI->form_validation->run() == TRUE)
        {
         $CI->load->model('page_update_model');
         $CI->page_update_model->sincity_content_booths($data);
         redirect(base_url().'manage_pages', 'location', 301);
        }
      }
    }
  }



How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]Aken[/eluser]
Most of the code in your library is fine, but libraries should not output directly or maneuver your app. Calling items like redirect() should be reserved for controllers.

Also, all controllers don't necessarily need to be "skinny". If your code is only used in that controller (AKA not repeated anywhere else), then it isn't a big deal. And that's very little code in the scheme of things (I have some WAY longer controllers).

I'm not going to get really specific with your code, because this isn't the time or place, and I don't do that for free. I'd worry less about the overall structure of your app and making it "pretty", and focus on being functional and working the way it's supposed to (for example, you're using $CI->image_lib->display_errors(); by itself, which is useless).


How can I get rid of this logic from the view? - El Forum - 01-31-2013

[eluser]behnampmdg3[/eluser]
[quote author="Aken" date="1359689089"]I don't do that for free.[/quote]
1 - How much do you charge me?
2 - What can you teach me?
Thanks