Properly using PHP Classes in CodeIgniter

Question

I have a class AdminMenu that I'm using to create the menu for the admin on my project. Here is the class:

class AdminMenu{
    public $menu = array(
        'Groups and Statuses' => 'groups',
        'Members Management' => 'members',
        'Book Manager' => 'books',
        'Activity Log' => 'activities',
        'Quiz Maker' => 'quiz',
        'Other' => 'other'
    );
    public function AdminMenu() {
        $html = "<nav><ul><li><input type='text' id='search' placeholder='Search' /></li>";    
        foreach($this->menu as $text => $link){
            $html .= '<li><a href="'. site_url(array('admin', $link)) .'">'. $text .'</a></li>';
        }
        $html .= '</ul></nav>';
        echo $html;
    }
}

As you can see, the constructor function is echo-ing out data, which is fine. So when I need the admin menu I can just call new AdminMenu(); and it appears great.

The thing is, CodeIgniter recommends keeping classes in the libraries directory and calling it like this : $this->load->library('adminMenu');. The problem is, for some reason it calls the constructor function here, so if I load it, then later create a new AdminMenu(), the constructor has been called a total of 2 times, so I have two menus.

Is loading of libraries meant to call the constructor and am I wrong here? Should I have created a different function for output? Or can I leave it in libraries and just call it like an included class?

Thanks!

Steven Moseley · Accepted Answer · 2013-05-23 01:28:26Z

4

CodeIgniter's loading will always create a single instance of your class in the local scope of your controller. That's just how they designed it. (I personally don't like that approach.)

Thus, if you're going to build your class as a CodeIgniter library, you'll want the constructor only to do build-up tasks (which is advisable in OO anyway), and have a separate method to retrieve the menu.

(Alternatively, you could load your class directly, using your own external loading, e.g. by adding an SPL autoloader).

Anyhow, try doing this:

class AdminMenu
{
    public $menu = array(
        'Groups and Statuses' => 'groups',
        'Members Management' => 'members',
        'Book Manager' => 'books',
        'Activity Log' => 'activities',
        'Quiz Maker' => 'quiz',
        'Other' => 'other'
    );

    public function __construct()
    {
    }

    public function getMenu()
    {
        $html = "<nav><ul><li><input type='text' id='search' placeholder='Search' /></li>";    
        foreach($this->menu as $text => $link){
            $html .= '<li><a href="'. site_url(array('admin', $link)) .'">'. $text .'</a></li>';
        }
        $html .= '</ul></nav>';
        return $html;
    }
}

And then use it like this:

$this->load->library('adminMenu');
echo $this->adminMenu->getMenu();

I also suggest makinng $menu private instead of public.

edited May 23, 2013 at 1:28

answered May 23, 2013 at 1:23

Steven Moseley

16.4k4 gold badges43 silver badges50 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

LexLythius Over a year ago

+1 on member privacy. By default, you should make members private (fields or methods) and make them protected or public only if you need it. Principle of implementation hiding.

Keith Over a year ago

thanks a ton for the help. I see about CI classes now, I'll keep my constructors only building, not outputting. and thanks for the input about private functions. __construct() would be public, right?

Steven Moseley Over a year ago

Your constructor always has to be public on non-static, non-Singleton classes. If you make it private, no external scope can instantiate it. In a Singleton pattern, you would have a private function __construct() and then a public function getInstance() which returns the private static self::$_instance (if instantiated) or sets self::$_instance = new __CLASS__().

LexLythius · Accepted Answer · 2013-05-23 01:22:11Z

2

CodeIgniter works with the Singleton approach. They instantiate your class automatically at load time.

You should rather use the existing instance, like this

$this->adminMenu->foo();

On the other hand, it's not advisable to do the output from the constructor. Constructors are meant to build the internal state of the object rather than carry out actions. Move your output to a method you can call.

One more thing: in PHP (unlike Java), it's recommended that your constructor be named __construct rather than using the class name. See this.

answered May 23, 2013 at 1:22

LexLythius

1,9441 gold badge13 silver badges20 bronze badges

1 Comment

Keith Over a year ago

thanks a ton for the info. and interesting about the constructor being named __construct. thanks again

Collectives™ on Stack Overflow

Properly using PHP Classes in CodeIgniter

2 Answers 2

3 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related