Welcome Guest, Not a member yet? Register   Sign In
Would like to condense some code.
#1

[eluser]Samuurai[/eluser]
Hi Everyone,

I'm trying to graduate from n00b to the next level up. At n00b level, you just want to get it working, which I have done, but this code i've written feels extremely bloated. Can someone please cast an experienced eye on this code and tell me how I can condense some of this duplicated code from my controller which handles contact information.

The view is set up to display values in the input areas if they're set, which is why there's lots of ternary operations there.

Code:
function contact()
    {
        $data->title = "Contact Us";
        $data->heading = "Contact Us";
        if($this->input->post('submitform'))
        {
            $this->form_validation->set_rules('name','Name', 'required');
            $this->form_validation->set_rules('email','Email', 'required|valid_email');
            $this->form_validation->set_rules('subject','Subject', 'required');
            $this->form_validation->set_rules('text','Message', 'required');
            
            if($this->form_validation->run())
            {
                $name = $this->input->post('name', TRUE);
                $email = $this->input->post('email', TRUE);
                $subject = $this->input->post('subject', TRUE);
                $text = $this->input->post('text', TRUE);
                $config['protocol'] = 'sendmail';
                $this->email->initialize($config);
                
                $this->email->from($email, $name);
                $this->email->to('[email protected]');
                $this->email->subject($subject);
                $this->email->message($text);
                $this->email->send();

                $this->session->set_flashdata('message', "Successfully sent email. We will get back to you as soon as possible.");

                redirect('site/home');
            }
            else
            {
                ($this->input->post('name')) ? $data->name = $this->input->post('name') : $data->name = '';
                ($this->input->post('email')) ? $data->email = $this->input->post('email') : $data->email = '';
                ($this->input->post('subject')) ? $data->subject = $this->input->post('subject') : $data->subject = '';
                ($this->input->post('text')) ? $data->text = $this->input->post('text') : $data->text = '';
                $this->load->view('header', $data);
                $this->load->view('contact', $data);
                $this->load->view('footer', $data);
            }
        }
        else
        {
            if($this->userlib->logged_in())
            {
                $this->load->model('User_model', 'user_m');
                $userid = $this->session->userdata('userid');
                $result = $this->user_m->name_and_email($userid);
                ($this->input->post('name')) ? $data->name = $this->input->post('name') : $data->name = $result['name'];
                ($this->input->post('email')) ? $data->email = $this->input->post('email') : $data->email = $result['email'];
                $this->load->view('header', $data);
                $this->load->view('contact', $data);
                $this->load->view('footer', $data);
            }
            else
            {
                ($this->input->post('name')) ? $data->name = $this->input->post('name') : $data->name = '';
                ($this->input->post('email')) ? $data->email = $this->input->post('email') : $data->email = '';
                ($this->input->post('subject')) ? $data->subject = $this->input->post('subject') : $data->subject = '';
                ($this->input->post('text')) ? $data->text = $this->input->post('text') : $data->text = '';
                $this->load->view('header', $data);
                $this->load->view('contact', $data);
                $this->load->view('footer', $data);
            }
        }
    }
}
edit -- Btw, I'm not after someone to rewrite my code for me, I'd just like it if someone could show me an approach that's a bit DRYer.
#2

[eluser]Damien K.[/eluser]
I'm a "noob" myself so take this for what it is worth. This should be self-explanatory with the embedded comments. It is not exactly how I would do it, but it gives the idea. HTH.

In your Contact controller:

Code:
function contact()
{
    ... [same] ...

    if($this->input->post('submitform'))
    {
        $this->_do_contact();
    }
    else
    {
        $this->_populate_contact();
    }

    // you may want to search the forum for various ways to handle this (ie, some mix of templating sub-system)
    $this->load->view('header', $data);
    $this->load->view('contact', $data);
    $this->load->view('footer', $data);
}

function _do_contact()
{
    $this->form_validation->set_rules('name','Name', 'required');
    $this->form_validation->set_rules('email','Email', 'required|valid_email');
    $this->form_validation->set_rules('subject','Subject', 'required');
    $this->form_validation->set_rules('text','Message', 'required');
    
    if($this->form_validation->run())
    {
        $contact = new Contact(); // I like to make a contact object to hold contact info

        $contact->name = $this->input->post('name', TRUE);
        $contact->email = $this->input->post('email', TRUE);
        $contact->subject = $this->input->post('subject', TRUE);
        $contact->text = $this->input->post('text', TRUE);

        $this->Contact_logic->send($contact);
        $this->session->set_flashdata('message', "Successfully sent email. We will get back to you as soon as possible.");
        redirect('site/home');
    }
}

function _populate_contact()
{
    if($this->userlib->logged_in())
    {
        $this->load->model('User_model', 'user_m');
        $userid = $this->session->userdata('userid');
        $result = $this->user_m->name_and_email($userid);

        // populate form with $result using another mean
        // I've extending CI to handle form population
        // you can check out the following forum post for an idea
        // http://ellislab.com/forums/viewthread/106052/#533725
    }
}

Contact_logic class:

Code:
class Contact_logic
{
    public function send($contact)
    {
        $config['protocol'] = 'sendmail'; // probably want to get this from a config file
        $this->email->initialize($config);
        
        $this->email->from($contact->email, $contact->name);
        $this->email->to('[email protected]'); // get your to address from a config file
        $this->email->subject($contact->subject);
        $this->email->message($contact->text);
        $this->email->send();

        return true; // proper error handling required
    }
}

EDIT: Look at 'form helper' on how to repopulate the form on input error.
#3

[eluser]Samuurai[/eluser]
Thanks very much for that! I am away till next wednesday and will have a proper look then and I'll let you know how I go.
#4

[eluser]Samuurai[/eluser]
This is brilliant! I'm just integrating it into my code now.

Why do you use the underscore notation on the _do_contact and _populate_contact functions?

Contact_logic goes into the libraries directory, right?

Thanks again!
#5

[eluser]mattpointblank[/eluser]
If I'm editing pre-populated values using the form_validation class, I do something like this:

Code:
$data['stuff'] = $this->Some_model->getStuff();

// then in my form view:

<input type="text" name="something" value="<?php echo set_value('something', $stuff['something']); ?>" />

This way, on first load, my page will show the database information, but if I modify it and submit the form again, it remembers my new value that was POSTed.

When I'm producing big, complex forms, I cheat a little and use the same form view to add and edit data. I use the above method, but add an @ sign in front of $stuff['something'] which means that in my controller, the edit() method can define $data['stuff'], but the add() one doesn't, and the error message is suppressed. It's a little hacky but it means that if I need to change my form, I don't have to modify it in two places (which gets complicated).
#6

[eluser]Samuurai[/eluser]
Yeah, I use the same form too. I use the form helper so my code looks like this:
I wish there was a condensed, shorthand way of typing:
Code:
<input type="text" name="subject" value="<?php if(isset($subject) ? echo $subject : echo ""))?>/>

What does set_value() do? - Can't find documentation on it.

//Edit - Found it: http://ellislab.com/codeigniter/user-gui...elper.html - thought it was a php method, not a CI method.
#7

[eluser]Damien K.[/eluser]
The underscore is there as per the manual--the reason is left as an exercise for the reader (Hint: see Controllers).

I put my Contact_logic somewhere under the libraries folder.

I guess you found the shorthand for (bool) ? (do1) : (do2). Many people do use mattpointblank's method for populating the form, and that is perfectly fine. However, I do not because somewhere and for some reason it didn't work well with me.

Furthermore, if you ever need another shorthand that is not supported you can always extend or create a new CI helper to fulfill your needs. I have one to handle dates in the view exactly the way I want it. My preference for this is to have as little PHP code in the view as possible.
#8

[eluser]Samuurai[/eluser]
[quote author="Damien K." date="1254426833"]
Code:
function _populate_contact()
{
    if($this->userlib->logged_in())
    {
        $this->load->model('User_model', 'user_m');
        $userid = $this->session->userdata('userid');
        $result = $this->user_m->name_and_email($userid);
    }
}
[/quote]

Just a quick one.. if I set anything in my $data object here, I find it's unavailable when I pass it to my views back in my index() function (which is in my Contact controller). I've not come up against this before.. how do I get the variable which is set lower down to be accessible earlier in my script?
#9

[eluser]BrianDHall[/eluser]
[quote author="Samuurai" date="1255110146"][quote author="Damien K." date="1254426833"]
Code:
function _populate_contact()
{
    if($this->userlib->logged_in())
    {
        $this->load->model('User_model', 'user_m');
        $userid = $this->session->userdata('userid');
        $result = $this->user_m->name_and_email($userid);
    }
}
[/quote]

Just a quick one.. if I set anything in my $data object here, I find it's unavailable when I pass it to my views back in my index() function (which is in my Contact controller). I've not come up against this before.. how do I get the variable which is set lower down to be accessible earlier in my script?[/quote]

You'll need to declare the $data variable as global at the start of the function, or my preference is to use such common variables as class properties, so I set var $data in the controller class and then access it anywhere in the controller with $this->data.
#10

[eluser]Samuurai[/eluser]
Thanks!
I think on my next site, I'll use the $this->data approach. I've used $data->variable SOO much on this site, I just wanna stay with this standard.

It doesn't seem to be working though..

here are the relevant functions:
Code:
function index()
    {
        $this->load->library('Contact_logic');
    $data->title = "Contact RaceDayStaff.com";
        $data->heading = "Contact RaceDayStaff.com";

        if($this->input->post('submit'))
        {
            $this->_do_contact();
        }
        else
        {
            $this->_populate_contact();
        }

        
        $this->load->view('header', $data); // a var_dump here shows only title and heading are set.
        $this->load->view('contact', $data);
        $this->load->view('footer', $data);
    }
And this:
Code:
function _populate_contact()
    {
        global $data; // Is this the right place to set it?
        if($this->userlib->logged_in())
        {
            $this->load->model('User_model', 'user_m');
            $userid = $this->session->userdata('userid');
            $result = $this->user_m->name_and_email($userid);
            ($this->input->post('name')) ? $data->name = $this->input->post('name') : $data->name = $result['name'];
            ($this->input->post('email')) ? $data->email = $this->input->post('email') : $data->email = $result['email'];
        }
        else
        {
            ($this->input->post('name')) ? $data->name = $this->input->post('name') : $data->name = '';
            ($this->input->post('email')) ? $data->email = $this->input->post('email') : $data->email = '';
            ($this->input->post('subject')) ? $data->subject = $this->input->post('subject') : $data->subject = '';
            ($this->input->post('text')) ? $data->text = $this->input->post('text') : $data->text = '';
        }
    }




Theme © iAndrew 2016 - Forum software by © MyBB