Compare 2 Different database Table

Question

I have php function that compares two different database table. Any suggestion on improving it.

     public function comparePartyTableCandidates()
   {
      $electionCandidates =  ElectionCandidate::all();
      foreach($electionCandidates as $electionCandidate )
      {
         $candidateName = $electionCandidate->CandidateFirstname. " ".$electionCandidate->CandidateLastname ;

         $result = Candidate::where('candidateName' ,'=',$candidateName )->first();

         if(empty($result))
        {

           $electionconstituency= ElectionConstituency::where('ConstituencyID', '=',$electionCandidate->ConstituencyID)->first();
           $constituency = Constituency::where('ConstituencyName', '=',strtolower($electionconstituency->ConstituencyName))->first();
            echo $electionconstituency->ConstituencyName;
            if(empty($constituency))
            {
                if((preg_match("/KINGSTON EAST & PORT ROYAL/",$electionconstituency->ConstituencyName)))
                {
                    $constituency = Constituency::where('ConstituencyName', 'like',"%"."kingston Eastern")->first();
                }
            }

           $electionparty = ElectionParty::where('PartyID', 'like', '%' . $electionCandidate->PartyID)->first();
           $politicalParty = PoliticalParty::where('partyName', 'like', '%' . $electionparty->PartyAbbr)->first();


           $candidate = new Candidate();
           $candidate->candidateName = $candidateName;
           $candidate->constituency_Id = $constituency->id;
           $candidate->party_id = $politicalParty->id;
           $candidate->save();
         }
      }
   }

Answer 1

TL;DR: Be clear, consistent and efficient, don't assume things and verify that the output of functions is as expected

---

1. Your code doesn't just compare candidates, it also creates new records. As such your function should be appropriately named e.g. compareAndCreateCandidates.

2. Comments are useful. You can declare a phpdoc style comment for your method and add comments to clarify any corner case e.g. the reasoning behind the if (empty($constituency)) code block.

3. Searches with 'LIKE', '%'. imply that unique identifiers are not being used. The % should be removed if the fields are unique. If not, you should find a unique field or a combination of fields that is unique for each entry and use that instead.

4. Two candidates in different parties and constituencies cannot share the same name i.e. a John Smith will not be created if another exists in the database. The criteria should be updated as in (3) above.

5. Since you are using laravel you should stick to the PSR-2 coding conventions e.g. for control structures:

   • There MUST be one space after the control structure keyword
   • There MUST NOT be a space after the opening parenthesis
   • There MUST NOT be a space before the closing parenthesis
   • There MUST be one space between the closing parenthesis and the opening brace
   • The structure body MUST be indented once
   • The closing brace MUST be on the next line after the body

6. Use strpos() instead of preg_match(). From the docs:

   Do not use preg_match() if you only want to check if one string is contained in another string. Use strpos() instead as it will be faster.

7. Use either single or double quotes consistently for plain strings:

   where('ConstituencyName', 'like',"%"."kingston Eastern")

8. Since you are using camelCase for variables make sure this is consistent e.g. $electionparty, $electionconstituency

9. The line echo $electionConstituency->ConstituencyName; looks like test code and should be removed. If not, then it violates the PSR-1 requirement that files should either declare something or have a side effect e.g outputting html. The ConstituencyNames should be saved in an array or string and returned by the function instead of echoing them.

10. Several variables are only used once and should not be initialized e.g $electionCandidates, $result

11. There is no check to ensure $electionParty and $politicalParty are not null.

12. The code assumes each model will be saved without error. The database saves should be wrapped in a transaction with proper error handling.

The code with some issues fixed is as follows:

/**
 * This is where a description of the method's purpose goes
 */

public function compareAndSaveCandidates()
{
    foreach (ElectionCandidate::all() as $electionCandidate) {
        $candidateName = $electionCandidate->CandidateFirstname . " " . $electionCandidate->CandidateLastname;
        if (!Candidate::where([
             ['candidateName', '=', $candidateName],
             //other criteria
        ])->first()) {
            $electionconstituency = ElectionConstituency::where('ConstituencyID', '=', $electionCandidate->ConstituencyID)->first();
            $constituency = Constituency::where('ConstituencyName', '=', strtolower($electionConstituency->ConstituencyName))->first();
            //echo $electionConstituency->ConstituencyName;

            //This is a corner case and deserves a comment
            if (!$constituency) {
                if ((strpos($electionConstituency->ConstituencyName, 'KINGSTON EAST & PORT ROYAL'))) {
                    $constituency = Constituency::where('ConstituencyName', 'like', '%' . 'kingston Eastern')->first();
                }
            }

            $electionParty = ElectionParty::where('PartyID', 'like', '%' . $electionCandidate->PartyID)->first();
            $politicalParty = PoliticalParty::where('partyName', 'like', '%' . $electionParty->PartyAbbr)->first();

            $candidate = new Candidate();
            $candidate->candidateName = $candidateName;
            $candidate->constituency_Id = $constituency->id;
            $candidate->party_id = $politicalParty->id;
            $candidate->save();
        }
    }
}