dcsimg
www.webdeveloper.com
Results 1 to 4 of 4

Thread: What is the best way forward?

  1. #1
    Join Date
    Apr 2016
    Posts
    2

    Angry What is the best way forward?

    Hi,

    I am busy redeveloping my in-house CMS and as far as possible try to follow best practices. I may not know all the best practices, and I may not use everybody's choice of framework, but given that I am an experienced developer from the days of old procedural style coding (my OOP needs work...), I do the best I can.

    I am currently in a scenario where I would like some input:

    According to SOLID principles, a function should have one function only. I am really struggling with that, and not sure how to make this better. My code is really feeling clumsy for me.

    Here is an example of code:

    HTML Code:
        public function add($uid)
        {
            if (empty($uid))
            {
                return false;
            }
            $data['userdata'] = $this->usercontact_model->get_user($uid);
            if (empty($data['userdata']))
            {
                return false;
            }
            else
            {
                $temp = $this->usercontact_model->get_contact_types();
                $temp_options = array();
                if (!empty($temp))
                {
                    foreach ($temp as $tk => $tv)
                    {
                        $temp_options[$tk]['value'] = $tv['id'];
                        $temp_options[$tk]['display'] = $tv['name'];
                    }
                }            
                $data['sidebar'] = array(
                    0 => array(
                        'type' => 'accordion',
                        'content' => array(
                            0 => array(
                                'icon' => 'settings',
                                'active' => 'active',
                                'heading' => 'Page actions',
                                'prompt' => lang('user_contacts_page_actions_head') . '<br />&nbsp;<br />',
                                'body' => '
                                    &raquo;&nbsp;<a href="' . base_url('user/usercontact/list_contacts') . '/' . $this->session->userdata('id') . '">' . lang('user_contacts_return_contacts') . '</a><br />
                                    &raquo;&nbsp;<a href="' . base_url('user/login/control_panel') . '">' . lang('user_contacts_return_cpanel') . '</a><br />
                                '
                            )
                        )
                    )               
                ); 
                $data['open_data'] = array(
                    'action' => '',
                    'method' => 'post',
                    'hidden' => array(
                        'user_id' => $this->session->userdata('id')
                    )
                );   
                $data['close_data'] = array(
                    'datepicker_init' => 'true',
                    'select_months' => 'true',
                    'select_years' => 15,
                    'format' => 'yyyy-mm-dd',
                    'clockpicker_init' => 'true'
                );        
                $data['contact_type_data'] = array(
                    'id' => 'contact_type_select',
                    'heading' => 'user_contact_type',
                    'options' => $temp_options
                );
                $data['active_data'] = array(
                    'id' => 'active_select',
                    'heading' => 'user_contact_active',
                    'options' => array(
                        0 => array(
                            'value' => 'Yes',
                            'display' => lang('user_contact_yes')
                        ),
                        1 => array(
                            'value' => 'No',
                            'display' => lang('user_contact_no')
                        )
                    )
                );            
                $data['value_data'] = array(
                    'id' => 'value',
                    'validate' => 'validate',
                    'placeholder' => 'Content',
                    'label' => 'user_contact_label_value'
                );
                $data['vacation_from_data'] = array(
                    'id' => 'vacation_from',
                    'placeholder' => 'Select vacation start date',
                    'label' => 'user_contact_vacation_from'
                );
                $data['vacation_to_data'] = array(
                    'id' => 'vacation_to',
                    'placeholder' => 'Select vacation end date',                
                    'label' => 'user_contact_vacation_to'
                );            
                $data['time_from_data'] = array(
                    'id' => 'time_from',
                    'placeholder' => 'Select available start time',                
                    'label' => 'user_contact_time_from'
                );
                $data['time_to_data'] = array(
                    'id' => 'time_to',
                    'placeholder' => 'Select available end time',                
                    'label' => 'user_contact_time_to'
                );  
                $data['submit_data'] = array(
                    'id' => 'submit_add',
                    'label' => lang('user_contact_save'),
                );                        
                $posts = $this->input->post();
                if (empty($posts))
                {
                    $this->core->render_view('template_admin', 'usercontact/add', $data);
                }
                else
                {
                    $this->core->render_view('template_admin', 'usercontact/add', $data);
                    preint($posts);                
                }
            }
        }
    I am using CodeIgniter 3.0.x and MaterializeCSS for the front-end.

    I can't help thinking that this function is very clumsy... I do not need comments on the code quality, but rather a solution to make it less clumsy. Any ideas? You can assume that the code works. I still have to build in form validation, hence this function will become even more clumsy if I don't take care of this now.

    I can probably do the array population for the form and sidebar elements in a separate function, but that is only a partial solution...

    Your thoughts, please?

    Kind regards,

    Kobus (Bliksem)

  2. #2
    Join Date
    Aug 2004
    Location
    Ankh-Morpork
    Posts
    21,037
    Based on what it seems to be doing (preparing a bunch of data for the view?), I might move that $data array into a class property, where you can populate it in one statement with all the static stuff:
    PHP Code:
    class whatever
    {
        private 
    $data = array(
            
    'open_data' => array(
                
    'action' => '',
                
    'method' => 'post',
                
    'hidden' => array(
                    
    'user_id' => $this->session->userdata('id')
                )
            ),
            
    'close_data' => array(
                
    'datepicker_init' => 'true',
                
    'select_months' => 'true',
                
    'select_years' => 15,
                
    'format' => 'yyyy-mm-dd',
                
    'clockpicker_init' => 'true'
            
    ),
            
    'etc' => array(
                
    'and' => 'so forth'
            
    )
        );

        public function 
    add($uid)
        {
            
    // do your checks and then modify $this->data as needed
        
    }

    Any parts of $this->data that are dynamic would be populated and/or modified within the add() method (or any other method that might also interact with that data).
    "Please give us a simple answer, so that we don't have to think, because if we think, we might find answers that don't fit the way we want the world to be."
    ~ Terry Pratchett in Nation

    How to Ask Questions the Smart Way (not affiliated with this site, but well worth reading)

  3. #3
    Join Date
    Apr 2016
    Posts
    2
    Thank you, NogDog. I have moved those declarations to separate functions so far. Now need to make them objects within the class - for now they are just normal arrays within methods. Your way seems better, but I have not yet gotten that far.

    B.

  4. #4
    Join Date
    Aug 2004
    Location
    Ankh-Morpork
    Posts
    21,037
    Yeah, without understanding what that data represents, I wasn't sure whether we're really looking at what should be multiple objects, and then the class/method in question would function as a sort of repository for them, passing an array of objects to the view, perhaps? Plus, it's been a few(?) years since I've done anything with CodeIgniter, and I don't remember how it's views prefer to receive data from the controller.
    "Please give us a simple answer, so that we don't have to think, because if we think, we might find answers that don't fit the way we want the world to be."
    ~ Terry Pratchett in Nation

    How to Ask Questions the Smart Way (not affiliated with this site, but well worth reading)

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  
HTML5 Development Center



Recent Articles