Welcome Guest, Not a member yet? Register   Sign In
Making things DRY. Some feedback please.
#1

[eluser]Bramme[/eluser]
Hey all,

I've got a question for the (maybe) more experienced CI user.

I'm looking for a best practice to make things more DRY:

I've got a little backend that handles news posting/editting. The posting and editting is done in two different methods but they look largely the same, seeing as they're both based on the same form (even the same view, I define the form action from within my controller), the procedure is highly similar etc... There's only a few minor differences.

Actually, here's the code:
Code:
function new_item()
{
    $this->auth->restrict();
    
    $this->load->library('validation');
    
    //rules    
    $this->validation->set_rules($rules);
    
    //fields    
    $this->validation->set_fields($fields);
    $this->validation->set_error_delimiters('<li>', '</li>');
    
    $standard_values['poster'] = 'MickM';
    $standard_values['type'] = 'news';
    
    if ($this->validation->run() == TRUE)
    {
        foreach ($fields as $field => $name)
        {
            $insert[$field] = $this->input->post($field);
        }
        
        $timestamp = time();
        $insert['datetime'] = unix_to_human($timestamp, TRUE, 'eu');
        $insert['type'] = $this->input->post('type');
        
        if($_FILES['thumb_url']['error'] != 4)
        {
            $upload_config['upload_path'] = './_assets/images/thumbs/';
            $upload_config['allowed_types'] = 'gif|jpg|png';
            $upload_config['max_size'] = '100';
            $upload_config['max_width'] = '150';
            $upload_config['max_height'] = '150';
            
            $this->load->library('upload', $upload_config);
            
            if ($this->upload->do_upload('thumb_url'))
            {
                $info = $this->upload->data();
                $insert['thumb_url'] =     '/_assets/images/thumbs/'.$info['file_name'];
            } else {
                echo $this->upload->display_errors();
                die();    
            }
        }
        else
        {
            $insert['thumb_url'] = '/_assets/images/thumbs/m.gif';
        }
        
        if ( ! $this->db->insert('news', $insert))
        {
            send_notice('error', 'Something went wrong with the query', 'self');
        }
        else
        {
            send_notice('success', 'Item successfully added', 'self');
        }
    }
    else
    {
        
        if ( ! empty($this->validation->error_string))
        {
            send_notice('validation_error', $this->validation->error_string);
            
            $standard_values['poster'] = $this->input->post('poster');
            $standard_values['type'] = $this->input->post('type');
            $standard_values['title'] = $this->input->post('title');
            $standard_values['content'] = $this->input->post('content');
        }
    }
    
    
    
    $data['form'] = $this->table_editor->create_form($standard_values);
    $data['action'] = '/admin/news/new_item';
    $this->template->write_view('region_content', '_admin/news/new', $data);
    $this->template->render();
}
I know it's rather lengthy, but it's just to give you an idea how similar it is!
#2

[eluser]Bramme[/eluser]
Code:
function edit($newsID = NULL)
{
    $this->auth->restrict();
    
    if (!isset($newsID))
    {
        redirect('/admin/news/overview');
    }
    
    $this->load->library('validation');
    
    //rules    
    $this->validation->set_rules($rules);
    
    //fields    
    $this->validation->set_fields($fields);
    $this->validation->set_error_delimiters('<li>', '</li>');
    
    // get values from db
    $this->db->where('newsID', $newsID);
    $qry = $this->db->get('news');
    $row = $qry->row();
    foreach ($fields as $field => $name)
    {
        $standard_values[$field] = $row->$field;
    }
    $standard_values['type'] = $row->type;
    
    if ($this->validation->run() == TRUE)
    {
        foreach ($fields as $field => $name)
        {
            $insert[$field] = $this->input->post($field);
        }
        
        $timestamp = time();
        $insert['datetime'] = unix_to_human($timestamp, TRUE, 'eu');
        $insert['type'] = $this->input->post('type');
        
        if ($this->input->post('chk') == 'yes')
        {
            if ($_FILES['thumb_url']['error'] != 4)
            {
                $upload_config['upload_path'] = './_assets/images/thumbs/';
                $upload_config['allowed_types'] = 'gif|jpg|png';
                $upload_config['max_size'] = '100';
                $upload_config['max_width'] = '150';
                $upload_config['max_height'] = '150';
                
                $this->load->library('upload', $upload_config);
                
                if ($this->upload->do_upload('thumb_url'))
                {
                    $info = $this->upload->data();
                    $insert['thumb_url'] =     '/_assets/images/thumbs/'.$info['file_name'];
                } else {
                    echo $this->upload->display_errors();
                    die();    
                }
            }
            else
            {
                $insert['thumb_url'] = '/_assets/images/thumbs/m.gif';
            }
        }
        
        $this->db->where('newsID', $newsID);
        if ( ! $this->db->update('news', $insert))
        {
            send_notice('error', 'Something went wrong with the query', 'self');
        }
        else
        {
            send_notice('success', 'Item successfully edited', 'self');
        }
    }
    else
    {
        
        if ( ! empty($this->validation->error_string))
        {
            send_notice('validation_error', $this->validation->error_string);
            
            $standard_values['poster'] = $this->input->post('poster');
            $standard_values['type'] = $this->input->post('type');
            $standard_values['title'] = $this->input->post('title');
            $standard_values['content'] = $this->input->post('content');
        }
    }
    
    $data['form'] = $this->table_editor->create_form($standard_values, TRUE);
    $data['action'] = '/admin/news/edit/'.$newsID;
    $this->template->add_js('_assets/js/jquery.js');
    $this->template->add_js('_assets/js/news_edit.js');
    $this->template->write_view('region_content', '_admin/news/new', $data);
    $this->template->render();
}
That's the second method.
#3

[eluser]xwero[/eluser]
Put the code in one method, route your new/edit urls to that method and handle the differences using the new/edit segment of the (rerouted) url.
#4

[eluser]nmweb[/eluser]
And use models for the db calls, thumbnailing etc.
#5

[eluser]Référencement Google[/eluser]
What's better from a MVC point of view about placing thumbnailing process, should this goes into a model or into a library (or helper in some cases) ?
#6

[eluser]drewbee[/eluser]
Library. As long as there are no database actions going on. I tend to cheat at this though and put queries within libraries. (not sure if this is correct or not). Seems redundant to me to seperate database actions outside of a library.
#7

[eluser]nmweb[/eluser]
Where did that idea come from that models only do database stuff? It's nonsense. Let me quote wikipedia on MVC
Quote:Model
The domain-specific representation of the information on which the application operates. Domain logic adds meaning to raw data (e.g., calculating whether today is the user's birthday, or the totals, taxes, and shipping charges for shopping cart items).
Many applications use a persistent storage mechanism (such as a database) to store data. MVC does not specifically mention the data access layer because it is understood to be underneath or encapsulated by the Model.
The data access layer is encapsulated by the model. You ask the model for the last ten articles and it gives them back to you, how is of no importance, be it a database, text, or even randomly generate ten articles. You should be able to change the data storage of your application just by changing the models.

Granted, libraries do not have a set place in MVC, but most of them probably resemble a model. Thumbnailing, i.e. representation of an image information is imo done in the model.
#8

[eluser]drewbee[/eluser]
[quote author="nmweb" date="1218235902"]Where did that idea come from that models only do database stuff? It's nonsense. Let me quote wikipedia on MVC
Quote:Model
The domain-specific representation of the information on which the application operates. Domain logic adds meaning to raw data (e.g., calculating whether today is the user's birthday, or the totals, taxes, and shipping charges for shopping cart items).
Many applications use a persistent storage mechanism (such as a database) to store data. MVC does not specifically mention the data access layer because it is understood to be underneath or encapsulated by the Model.
The data access layer is encapsulated by the model. You ask the model for the last ten articles and it gives them back to you, how is of no importance, be it a database, text, or even randomly generate ten articles. You should be able to change the data storage of your application just by changing the models.

Granted, libraries do not have a set place in MVC, but most of them probably resemble a model. Thumbnailing, i.e. representation of an image information is imo done in the model.[/quote]

With that being said then, I would infer that retrieving the images / putting the images should be done in the model. However any manipulation to the images should be done in the controller.

Libraries are a bit weird. As I stated I tend to do all my database stuff within the library as well. Though phasing it out to a model is feasible, it seems rather pointless to me. How does everyone else handle libraries?
#9

[eluser]Référencement Google[/eluser]
I personally do all image manipulation in a library. To give more precision to my answer, I extend the CI Image manipulation library in a MY_Image.php file.

I can understand the point of doing that in a model, but for keeping my code clean I prefer not to explode parts of the code everywhere and I prefer to keep some grouping of related things.
#10

[eluser]wiredesignz[/eluser]
To backup what nmweb is saying.

An image is data, wether it comes as BLOB from a table or from a file or created dynamically, it is merely a binary (or hex) representation of something physical. It's not until we add appropriate headers to it that it becomes a view.

That being the case a model would be the logical MVC choice to manage image data.

The controller should not be given this job ever.

Using a library is merely shifting the job to an object that does the job of a model. (differemt name same job).




Theme © iAndrew 2016 - Forum software by © MyBB