Controller method optimization

Question

I have the following function for validating users facebook information against the rules setup in the database for users. Its working fine but i need to know if it can be more optimized. Few things that you need to know are:

1. There is a possibility that particular rule would not exists in db.
2. But if exists and does not match with user details it must redirect to error page.
3. it is also possible that user has not set the information in their profile so in this case it should redirect to error page.

4. my dbdetails array has following structure is

   Array( ['Demography'] => Array( ['Rule'] => Array( [0] => 'Some Value' [1] => 'Some Value'... ) ['Value'] => Array( [0] => 'Some Value' [1] => 'Some Value' ) ) )

And the fbdata is exactly same what facebook returns when user visits the page after allowing the application. Here is my function which validates/

private function validate_demographics($dbDetails, $fbData)
    {
        $this->load->library('facebook', array('appId' => APP_ID, 'secret' => APP_SECRET));

        //Is page like restriction is there
        if( in_array('People Who Like My Page', $dbDetails['Demography']['Value']) )
        {
            //Checks whether user has liked the page or not
            if( !$fbData['page']['liked'] ) {
                //If user has not liked the page set the network flag to 1.
                redirect('viewer/demographics_not_match/1');
            }
        }

        if( in_array('Age', $dbDetails['Demography']['Rule']) ) {
            $ageInd = array_search('Age', $dbDetails['Demography']['Rule']);
            $validAge = explode('-', $dbDetails['Demography']['Value'][$ageInd]);
            $userBirthdate = explode('/',$this->viewer_fb_data['birthday']);
            $userAge = date('Y') - $userBirthdate[2];
            if( $userAge >= $validAge[0] && $userAge <= $validAge[1] ) {
                return true;
            } else {
                redirect('viewer/demographics_not_match/2');
            }
        }

        if( in_array('High-School', $dbDetails['Demography']['Rule']) or in_array('College', $dbDetails['Demography']['Rule']) ) {
            //Checks whether user has set his educational details on facebook
            if( array_key_exists('education', $this->viewer_fb_data) ) {
                foreach( $this->viewer_fb_data['education'] as $userEducation )
                {
                    if( in_array($userEducation['school']['name'], $dbDetails['Demography']['Value']) ) {
                        return true;
                    }
                }
            } else {
                redirect('viewer/demographics_not_match/3');
            }
        }

        if( in_array('City', $dbDetails['Demography']['Rule']) or in_array('State', $dbDetails['Demography']['Rule']) or in_array('Country', $dbDetails['Demography']['Rule']) )
        {
            //Checks whether user has set his location on facebook
            if( array_key_exists('location', $this->viewer_fb_data) ) {
                $userLocation = explode(',', $this->viewer_fb_data['location']['name']);
                if( in_array($userLocation[0], $dbDetails['Demography']['Value']) ) {
                    return true;
                }
            } else {
                redirect('viewer/demographics_not_match/5');
            }
        }

        if( in_array('Work-Place', $dbDetails['Demography']['Rule']) )
        {
            //Checks whether user has set his work details on facebook
            if( array_key_exists('work', $this->viewer_fb_data) ) {
                $workInd = array_search('Work-Place', $dbDetails['Demography']['Rule']);
                $workPlaces = explode(',', $dbDetails['Demography']['Value'][$workInd]);
                foreach( $this->viewer_fb_data as $fbWork )
                {
                    if( in_array($fbWork['employer']['name'], $workPlaces) ) {
                        return true;
                    }
                }
            } else {
                redirect('viewer/demographics_not_match/4');
            }
        }
        return false;
    }

Answer 1

The code looks OK, and what is probably going to take you time is the call to the Facebook API, which means there's no need to optimize that snippet.

I also have two unimportant remarks:

• Your function either returns true/false, or redirects to another page. There is no reasonable way to guess that (given a name such as validate_demographics). You should be careful when you are redirecting in some cases but not all of them. This is a lack of consistency that could prove dangerous afterwards (eg. if you put a redirect after this is executed, it won't work in every case).
• The validation method is a bit surprising: as soon as you found a rule that is validated, your return TRUE. Don't you want to check that all the rules match?

---

You want to consider it valid when one of the rules matches, and invalid when all them don't match. If nothing matches, where do you want to redirect the user? #1? #4?

To solve the "does one rule match?" problem, I would set a variable $nothing_matches to TRUE at the beginning of the function, and change every return TRUE to $nothing_matches = FALSE. You can simply return $nothing_matches; at the end of the function.

Now, if nothing matched (`$nothing_matched is TRUE) you can hardcode the rule you want to redirect to, since there's no better way to choose the rule.