HiveBrain v1.2.0
Get Started
← Back to all entries
principlephpMinor

Best practice for generating jQuery dynamical content

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
practicedynamicalgeneratingjqueryforcontentbest

Problem

I am new user of CodeIgniter and I am trying to build an application that there are lots of jQuery dynamical content.

Below I provide a code that I am using in order to be precise. The code below is running and I cannot see any problem. However, since I will be using pieces of code like this in several different parts of my application, I would like to know if this is a good design. If it is not, please let me know how to proceed.

The code below basically describe a search form in which the results are dynamically included.

This is my view:


    
        
        
            $(document).ready(function() {
                $("form#searchForm").submit(function() {
                    var theCity = $("select#chooseCity").val(); 
                    $.post("welcome/searchPeopleResults/", {theCity: theCity}, function(data) {
                        $("div#searchResults").html(data);
                    });
                  return false
                });
            });
        
    

    

        
            Selecione uma cidade:             
            
                
                require($thePathDataFiles.$theCitiesOptionsHTML);                
                ?> 
            
        
        
            
                

     


This is my controller:

class Welcome extends CI_Controller {

        public function index()
        {
                    $this->load->view('searchPeople');
                    $this->load->view('css/format');
        }
            public function searchPeopleResults(){
                $theCity=$this->input->post('theCity');
                $this->load->model('MSearchPeople');
                $data=$this->MSearchPeople->provideSearchPeopleResults($theCity);
                echo $data;
            }
    }


This is my model:

```
Class MSearchPeople extends CI_Model {

function provideSearchPeopleResults($theCity) {

// Here I got my database query ($theUsername, $theFirstName, $theLastName, $theSumma

Solution

You are right with your concerns regarding the model rendering the output. But you're almost there ain't you? Why don't you let your model return the data instead of the HTML-Code and let the controller call writeHtmlSearchPeople:

class MSearchPeople extends CI_Model {
    function provideSearchPeopleResults($theCity) {
        $result = // db query, normalize data,
        return $result;
    }
}

class Welcome extends CI_Controller {
    public function searchPeopleResults() {
        $theCity = $this->input->post('theCity');
        $this->load->model('MSearchPeople');
        $data = $this->MSearchPeople->provideSearchPeopleResults($theCity);
        echo writeHtmlSearchPeople($data); // or however you rendered the data
    }
}


Now this the controller acts as the mediator between your view and your model. In my opinion it is important that the controller never contains any view logic besides returning or echo-ing it. If the data from your model requires iterations, put these in your view.

Other remarks:

  • I don't know CI, yet you might want to check to validate your input


($theCity) for invalid values and react accordingly

  • Code Style:



  • Class MSearchPeople, the class keyword probably should be lower-case



  • Indentation: might be a copy & paste problem, though try to keep it consistent.



  • Whitespaces in assignments: More of a personal thing though widely used: a whitespace before and after an assignment operator: $theCity = $this->input->post('theCity');. Makes it easier to read.



  • Variable & class naming: probably the second most difficult things in programming (after writing comments). Maybe you can find better names for your variables and names? Some examples:



  • Welcome Controller? What is it's responsibility? I really can't tell in what way it has to do with welcoming whatever being.



  • $theCity=$this->input->post('theCity'); This name indicates for me a one valid city entity. But rather more ain't it the current $searchTerm? In general I'd consider variables starting the the very bad practice. The in a name would indicate the final single entity resulted from a long journey. But even then I'd skip the the as it puts too much emphasis on the the and not on the following object. I'd go with $matchingCities instead.



  • I'd consider the hungarian notation obsolete in typed languages (MSearchPeople => SearchPeople) (bluntly assumung that the M stands for Model).



  • provideSearchPeopleResults this name indicates that this information is made available by other means and not returned. The common pattern when returning data is get -> getSearchPeopleResults. As this function is called on the MSearchPeople model, the SearchPeople part in this function name is duplicate information too. The function name could be shortened to search.



  • While I only know very little of your code, I'd consider search part of either the People model or better, repository. There is no need for a dedicated model for searching. Of course this highly depends on the business model you are modeling.

Code Snippets

class MSearchPeople extends CI_Model {
    function provideSearchPeopleResults($theCity) {
        $result = // db query, normalize data,
        return $result;
    }
}

class Welcome extends CI_Controller {
    public function searchPeopleResults() {
        $theCity = $this->input->post('theCity');
        $this->load->model('MSearchPeople');
        $data = $this->MSearchPeople->provideSearchPeopleResults($theCity);
        echo writeHtmlSearchPeople($data); // or however you rendered the data
    }
}

Context

StackExchange Code Review Q#41959, answer score: 2

Revisions (0)

No revisions yet.