Forms (show and process) on the same controller or on different ones?

The question is - what is the best approach 
to show froms (if post/get is empty) and process data(if post/get contains data)  in one controller or to make different controllers(or in this particular case methods), and if data don't pass the validation redirect back to previous one?

Which approach is better? 
If you do all in one controller(method) it become a mess, with lots of IF statements and curly bracers scattered all over the place☻

I keep everything in one controller and have a separate method for create, read, update and delete.
If the validation fail they should get to the same function/method for displaying error messages. If it works you can then redirect them. As this example, I redirect them to a specific update function that let's the user set more information about that group.

Controller:

PHP Code:

<?php
defined('BASEPATH') OR exit('No direct script access allowed');

class Groups extends User_Controller
{
    public function create()
    {
        $this->data['groups_type'] = array(
            'position'   => lang('groups_type_position'),
            'department' => lang('groups_type_department'),
            'security'   => lang('groups_type_security'),
        );
        
        $this->load->helper('form');
        $this->load->library('form_validation');
        $this->config->load('form_validation');
        $validation_rules = $this->_get_rules();

        $this->form_validation->set_rules( $validation_rules );
        if( $this->form_validation->run() === TRUE )
        {
            $group_id = UUIDv4();
            
            if ( $this->group_model->create( $group_id ) === TRUE )
            {
                redirect('admin/groups/update_' . $this->group_model->_get_groups_type() . '/' . $group_id);
            }
        }
        else
        {
            $this->load->view('admin/groups/create',$this->data);
        }
    }
    
    public function _is_unique()
    {
        return $this->group_model->is_unique();
    }
    
    private function _get_rules()
    {
        return array(
            array(
                'field' => 'groups_name',
                'label' => lang('groups_name'),
                'rules' => array(
                    'trim',
                    'required',
                    'min_length[3]',
                    'max_length[80]',
                    'regex_match['.$this->config->item('error_alpha_numeric_spaces_dash_colon').']',
                    array('is_unique', array( $this, '_is_unique' ) )
                ),
                'errors' => array(
                    'regex_match' => lang('groups_name') . lang('error_alpha_numeric_spaces_dash_colon')
                )
            ),
            array(
                'field' => 'groups_type',
                'label' => lang('groups_type'),
                'rules' => array(
                    'trim',
                    'required',
                    'in_list['.implode(array_keys($this->data['groups_type']),',').']'
                )
            )
        );
    }
    
} 


Model:

PHP Code:

<?php
class Group_model extends MY_Model
{
    
    public function _get_groups_name()
    {
        return trim($this->input->post('groups_name'));
    }
    
    public function _get_groups_type()
    {
        return trim($this->input->post('groups_type'));
    }

    private function _get_data()
    {
        $data = array(
            'name'        => $this->_get_groups_name(),
            'type'        => $this->_get_groups_type(),
        );
        return $data;
    }
    
    public function create( $group_id )
    {
        $data = $this->_get_data();
        $data['company_id'] = UUID_TO_BIN($this->auth_company_id);
        $data['group_id']   = UUID_TO_BIN($group_id);
        return $this->db->insert($this->db_table('groups'),$data);
    }
    
    public function is_unique()
    {
        $company_id = UUID_TO_BIN($this->auth_company_id);
        
        $q = $this->db
                ->select('group_id')
                ->where('company_id',$company_id)
                ->where('name',$this->_get_groups_name())
                ->where('type',$this->_get_groups_type())
                ->get($this->db_table('groups'));

        if( $q->row() )
        {
            return FALSE;
        }
        return TRUE;
    }
} 


One Controller - Different Methods. Name the methods really clearly so you understand what is going on. Like showNewRegisterForm() shows a form that has not been filled in yet. validateSubmittedRegisterForm() - validates the form. If it fails validation then don't bury a bunch of IF ELSE code - send it to a new method called something like showValidationFailedForRegisterForm()
If it passes validation - again don't bury the next step in the same method - make a new method for whatever the next step is. This keeps your methods more compact and much easier to maintain.
Also the initial url for the form call has to be public - but all your other methods in the controller can be private which helps security. One pattern is to have the form address be the default index method of the controller. In that index method do some basic checks to confirm your form has been submitted etc - and then route to the appropriate private method. You can also add a hidden field and value to your form - and check for it first. If that value is not there, then its probably some kind of bot so there is no need to validate the form at all.

Both the previous answers are contain good ideas IMHO.

I had been doing it all in one method, before the if/else fiasco finally wound me up enough that I thought it would just be easier to have two separate views and methods for update or create. (Although I would definitely keep it all in one controller). The additional repeated code is not too substantial. A pain though when the form keeps changing, having to update two views and two methods.

The back button on the browser causes lots of work/problems as well, still have not found a reliable and cross-browser way to prevent the 'do you want to resend form data' browser message etc.

Paul.

I haven't done so myself; But you can add some logic to a add_edit view so that it post to /save/id-here-if-available and save() then looks for an id-here-if-available. If it exists (and in database), call update() else create() and take the logic from there. So that you get rid of the big IF/ELSE.

As you will probably use set_value() you will need to make sure that you create empty sets in the create() function for the second parameter.

Regarding 'do you want to resend form data' you will need to save all post data in $_SESSION and redirect the user. And use that $_SESSION data after the redirection have finished.
https://en.wikipedia.org/wiki/Post/Redirect/Get

You can always extend default controller with new functionality. Codeigniter's documentation is the good place to start.

Here is a CRUD Controller Template for what they are talking about above.

PHP Code:

<?php
defined('BASEPATH') OR exit('No direct script access allowed');

/**
 * -----------------------------------------------------------------------
 * Editor   : PhpStorm
 * Date     : 5/21/2018
 * Time     : 6:14 AM
 * Authors  : Raymond L King Sr.
 * -----------------------------------------------------------------------
 *
 * Class        NameController
 *
 * @project     ci3fm
 * @author      Raymond L King Sr.
 * @link        http://www.procoversfx.com
 * @copyright   Copyright (c) 2009 - 2018 Custom Software Designers, LLC.
 * @license     http://www.procoversfx.com/license
 * -----------------------------------------------------------------------
 */
class NameController extends CI_Controller
{

    /**
     * Class properties - public, private, protected and static.
     * -------------------------------------------------------------------
     */

    /**
     * __construct ()
     * -------------------------------------------------------------------
     *
     * Class    Constructor
     *
     * NOTE: Not needed if not setting values or extending a Class.
     */
    public function __construct()
    {
        parent::__construct();

        log_message('debug', "NameController Controller Class Initialized");
    }

    /**
     * index ()
     * -------------------------------------------------------------------
     *
     */
    public function index()
    {

    }

    /**
     * create ()
     * -------------------------------------------------------------------
     *
     */
    public function create()
    {

    }

    /**
     * edit ()
     * -------------------------------------------------------------------
     *
     * @param  int $id
     */
    public function edit(int $id = 0)
    {

    }

    /**
     * update ()
     * -------------------------------------------------------------------
     *
     * @param  int $id
     */
    public function update(int $id = 0)
    {

    }

    /**
     * delete ()
     * -------------------------------------------------------------------
     *
     * @param  int $id
     */
    public function delete(int $id = 0)
    {

    }

    /**
     * manage ()
     * -------------------------------------------------------------------
     *
     */
    public function manage()
    {

    }


    // -------------------------------------------------------------------

}   // End of NameController Controller Class.


/**
 * -----------------------------------------------------------------------
 * Filename: NameController.php
 * Location: ./application/controllers/NameController.php
 * -----------------------------------------------------------------------
 */ 


There's a good start.

What did you Try? What did you Get? What did you Expect?

Joined CodeIgniter Community 2009.  ( Skype: insitfx )