Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The same view (a layout) used for all pages in Kohana3.3 project. That's why it could be placed in before() method instead of every action. Right?

Is there any better way to optimize the code below so you shouldn't use so many $this->? Are there any other improvements of this code so it looks really good?

public function before()
{   //shows layout for all pages
    $this->view = View::factory('layout');
    $this->view->left_column='pass this string to layout';
}

public function action_index()
{   //index section on the page
    $this->view->content = View::factory('index');
    $this->view->content->text = 'some index text';
    $this->response->body($this->view);
}

public function action_about()
{
    //about section on the page
    $this->view->content = View::factory('about');
    $this->view->content->text = 'some about text';
    $this->response->body($this->view);
}
share|improve this question

migrated from stackoverflow.com Jan 2 '13 at 16:56

This question came from our site for professional and enthusiast programmers.

1  
I don't get the question, your code is good. And $this refers to the object that it's in so why would you use less of it? Code is good. –  Nathan Sakoetoe Dec 29 '12 at 19:15
    
Thank you for your comment! Now I'm sure I'm on the right track! –  Haradzieniec Dec 30 '12 at 10:05

2 Answers 2

up vote 1 down vote accepted

Your code is violating the "Don't Repeat Yourself" (DRY) Principle, which is why it seems like you have "too many" $this->* lines. As the name implies, your code should not repeat. Usually, in an instance such as this, I would say to use the constructor and inject those values you need; However, I don't know for sure if the class you provided is also a factory, in which case you would use a wrapper method.

//non-factory method
public function __construct( $view, $text ) {
    $this->view->content = View::factory( $view );
    $this->view->content->text = $text;
    $this->response->body( $this->view );
}

//no other methods need, simply inject while instantiating

//factory method
public function render( $view, $text ) {
    $this->view->content = View::factory( $view );
    $this->view->content->text = $text;
    $this->response->body( $this->view );
}

public function action_index() {
    $this->render( 'index', 'some index text' );
}
share|improve this answer

You need extends from Controller Template like this

class Controller_Main extends Controller_Template {
   //This it the main html container i suggest use html5boilerplaite
   public function $_template = 'layout'; 

   public function action_index() {
      $data = array();
      $this->template->content = View::factory('frontend/home',$data);
   }
}

class Controller_Profile extends Controller_Main{

   public function action_index() {
      $data = array();
      $this->template->content = View::factory('frontend/profile',$data);
   }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.