[Beginner] Controller issue

[eluser]Ashley Snowdon[/eluser]
Hello,

I've just begun my experimentation with CodeIgniter, and I am trying to code my own user management.

I'm aware of the way the URI works, /class/function/params

I'm trying to create the following: /users/register, /users/login (and in the future /users/reminder/password, etc).

The way I envisaged this working was to have a 'users.php', but then I released that this class could get very messy. I then decided it would be neater to see if it was possible to create a subfolder in the controllers directory called 'users', then have a 'register.php', 'login.php' and build my authentication and validation from here.

Is this a good way to accomplish this? I went ahead and attempted to do it, but I'm having issues with loading a class when just /users is accessed.

Any help or guidance would be appreciated as I am fairly new to the MVC architecture.

Regards,
Ashley

[eluser]n0xie[/eluser]

The way I envisaged this working was to have a ‘users.php’, but then I released that this class could get very messy.

Why did you think it would get messy. It would only contain a few functions, not more. Would be pretty clean.

[eluser]WanWizard[/eluser]
It's all a matter of preference. You could even mix and match, use a single controller for a couple of simple methods, and split into controllers in a directory when it becomes complicated.

You can catch the requests for '/users' using a route, and route the request to something like 'users/overview'...

[eluser]Ashley Snowdon[/eluser]
[quote author="n0xie" date="1273601391"]

The way I envisaged this working was to have a ‘users.php’, but then I released that this class could get very messy.

Why did you think it would get messy. It would only contain a few functions, not more. Would be pretty clean.[/quote]

When I would have my authentication, checking if they're logged in, log a user out, password reset, check if username and email already exist, etc, etc.. I figured the class would get pretty hefty. Not to mention that the class:users and function:createRegistration would need to rely on a lot of other functions within the "register" scenario - where would these go?

[quote author="WanWizard" date="1273601438"]It's all a matter of preference. You could even mix and match, use a single controller for a couple of simple methods, and split into controllers in a directory when it becomes complicated.

You can catch the requests for '/users' using a route, and route the request to something like 'users/overview'...[/quote]

Can you give an example of when I would need a single controller and when I would need to split the controllers into a directory?

I realise now that I could catch the requests and route it to another class, thanks for this.

[eluser]n0xie[/eluser]
[quote author="Ashley Snowdon" date="1273601948"][quote author="n0xie" date="1273601391"]

The way I envisaged this working was to have a ‘users.php’, but then I released that this class could get very messy.

Why did you think it would get messy. It would only contain a few functions, not more. Would be pretty clean.[/quote]

When I would have my authentication, checking if they're logged in, log a user out, password reset, check if username and email already exist, etc, etc.. I figured the class would get pretty hefty. Not to mention that the class:users and function:createRegistration would need to rely on a lot of other functions within the "register" scenario - where would these go?[/quote]
Obviously you do all the 'stuff' either in a library or a model. The controller only controls the 'flow' of the application. Basically 'If this, do this, else do that'. The implementation you hide away. (also called Fat Models, Skinny Controllers principle).

Take a look at Ion's Auth example controller for their Authentication Library. Most functions have about 10-20 lines of code, and most of the code only dictates the flow of the application: the controller only controls what happens when, not how it happens.

[eluser]WanWizard[/eluser]
It's difficult to give examples, as it is entirely up to you want you think is managable, and what not.

In my project I don't use the single uri to controller mapping that standard CI uses, but a very modular approach, in which I try to keep all functionality that logically belongs together in a single controller. Which means I have modules with a single controller with 2000 lines, and modules with ten controllers with only a few methods.

As for authentication, I think that most people extend the Controller library, and do their security checks in the constructor of MY_Controller. This ensures that it is automatically executed whenever a controller is loaded.

[eluser]Buso[/eluser]
a single controller will do just fine.. 5-10 methods are not too many

in fact, i think that splitting it would be messy. Where would 'logout' go? In the login controller, or the registration controller?

[eluser]Ashley Snowdon[/eluser]
Thanks everyone for your comments. It seems that I have greatly misunderstood the concept of a controller. A controller is to tell the application WHAT to do, not HOW to do it. (Correct?)

I have ~50 line bit of code that I run inside a function within a controller that updates a database table, without showing any views - it's run as an automatic cronjob, not via a human. Does this mean I've placed this in the wrong area? Should this have gone in a model?

Here is the code below (application/controllers/cron.php):

Code:

<?php

class Cron extends Controller
{
    var $fsock_timeout    = 5;
    var $max_attempts    = 8;
    var $update_twitter    = false;
    
    function __construct()
    {
        parent::Controller();
    }
    
    public function index()
    {
        //
    }
    
    public function cron1()
    {
        //Cron 1
        $this->load->model('servers_model');
        
        $servers = $this->servers_model->getServers();
        
        foreach ($servers as $server) {
            if ($server->ip != '') {
                
                //Add a retry protection loop to prevent temp failed responses from the server
                //Sometimes we get a timeout when the server is actually online
                $count = 1;
                $loop = true;
                
                while ($loop) {
                    $start    = microtime(true);
                    $socket    = fsockopen($server->ip, $server->port, $errno, $errstr, $this->fsock_timeout);
                    $stop    = microtime(true);
                    $ping    = floor(($stop - $start) * 1000);
                    
                    //If it got a connection fine or max tries, then break out of loop
                    if (($socket) || ($errno == '111') || ($count >= $this->max_attempts)) {
                        $loop = false;
                    }
                    
                    $count += 1;
                }
                
                if ($socket) {
                    $status = 'online';
                    fclose($socket);
                } else {
                    $ping = -1;
                    //110 = timeout, 111 = refused
                    if ($error == '111') {
                        $status = 'offline';
                    } else {
                        $status = 'timeout';
                    }
                }
            } else {
                $status = 'offline';
            }
            
            $data = array(
                'status'    => $status,
                'ping'        => $ping
            );
            
            $this->servers_model->updateServer($server->id, $data);
            
            if (($server->status != $status) && ($this->update_twitter)) {
                //@todo Update Twitter
            }
        }
    }
    
    public function cron2()
    {
        //Cron 2
    }
    
    public function cron3()
    {
        //Cron 3
    }
}

[quote author="WanWizard" date="1273603560"]As for authentication, I think that most people extend the Controller library, and do their security checks in the constructor of MY_Controller. This ensures that it is automatically executed whenever a controller is loaded.[/quote]

I don't know what "MY_Controller" is, so I'll look this up in the documentation now.

Many thanks.

[eluser]Buso[/eluser]
[quote author="Ashley Snowdon" date="1273615118"]

I don't know what "MY_Controller" is, so I'll look this up in the documentation now.

Many thanks.[/quote]

in case you haven't found it yet: http://codeignitor.com/user_guide/genera...aries.html (extending native libraries)

[eluser]n0xie[/eluser]
[quote author="Ashley Snowdon" date="1273615118"]Thanks everyone for your comments. It seems that I have greatly misunderstood the concept of a controller. A controller is to tell the application WHAT to do, not HOW to do it. (Correct?)[/quote]
Well CodeIgniter doesn't force this on you but in my opinion, yes, a controller should only 'control' flow of the application.

[quote author="Ashley Snowdon" date="1273615118"]
Does this mean I've placed this in the wrong area? Should this have gone in a model?
[/quote]
There isn't really a 'wrong' approach as long as it works. People tend to forget that a framework is just a set of tools to get some job done. If it does the job it's fine. But for the sake of this argument, I would have done it a little different. This is just an example in non working pseudo code, and it might not be the best solution for your particular problem, but it might give you insight in how you can keep your Controller small and move most of your code to a model or library:

Code:

<?php

class Cron extends Controller
{
    function __construct()
    {
        parent::Controller();
    }
    
    public function cron1()
    {
        //Cron 1 we only need a couple of lines
        $this->load->model('servers_model');
        
        $this->servers_model->update_status_all();
        $list = $this->servers_model->get_status('offline');
        
        if ($list !== FALSE)
        {
           $this->load->library('sometwitterlib');
           $this->sometwitterlib->update($list)
        }
    }
}

// Model: let's break it up into small pieces and have 1 function delegate

class Servers_model extends Model
{
    function update_status_all()
    {
      $list = $this->get_list();
      foreach ($list as $server)
      {
         $status = $this->check_status($server);
         $this->set_status($server)
      }
    }

    function get_list() { // returns a list of server, most likely database }
    function check_status() { // returns TRUE if online, FALSE if offline }
    function set_status() { // sets server status, most likely database }
    function get_status() { // returns a list of online or offline servers, most likely database }
    function update_status($server_id) { // bonus: updates the status of 1 specific server }

Now the advantage to doing it this way is that you can re-use all the functions in the model. For instance in a admin panel you might want to show a list of online servers. You can use the same get_status('online') for this. Or you want a list of all your servers, you can use get_list. You want to check the status of a particular function realtime? Just use check_status. Or you might want to have a realtime update of your servers. You can call the delegation function update_status_all() to update them all, or update_status() to update just one.

You get all these extra functions for free just by removing the code from the Controller, breaking it up in little pieces, and then move it into a model or library.