Zip code reduce function

Question

My task is to write a function that would take an array of zip codes and spit out only the zip codes that do not qualify. A non-qualifying zip code will not exist in the database and does NOT have other input zip codes within 140 miles.

For example, if I input 90210, 90211, 77389 into the function, it should only spit out 77389 since it is not close to 90210 and 90211. Both 90210 and 90211 are disqualifying zip codes because they are close to one another.

Here are the two functions I am using:

//Accepts an input array of zips and outputs an array of zips that are not in proximity of any other input zip by a predefined distance

    protected function qualifyZips($zips, $radius=70){
        $validInputZips = array();
        //looping through input zips to validate them against the zip master table
        foreach ($zips as $key => $zip) {
            $inputZip = ZipCode::where('zip', $zip)->first(); //some zip data from DB
            //While we are at it, let's grab the lat/lon
            if ($inputZip) {
                $validInputZips[$zip]['latitude'] = $inputZip->latitude;
                $validInputZips[$zip]['longitude'] = $inputZip->longitude;
                $validInputZips[$zip]['hits'] = 0;
            }
        }

       $maskedInputZips = $validInputZips;
       $outputArray = array();

        foreach ($validInputZips as $zip => $latlon) {

            foreach ($maskedInputZips as $maskedZip => $maskedLatlon) {

                if ($zip != $maskedZip) {
                    $miles = $this->haversineGreatCircleDistance($latlon['latitude'], $latlon['longitude'], $maskedLatlon['latitude'], $maskedLatlon['longitude'], 3959);

                    if ($miles < ($radius * 2)) {
                        $validInputZips[$zip]['hits'] += 1;
                    }
                }
            }
            if ($validInputZips[$zip]['hits'] == 0){
                $outputArray[] = $zip;
            }
        }



        return $outputArray;
    }


   protected function haversineGreatCircleDistance($latitudeFrom, $longitudeFrom, $latitudeTo, $longitudeTo, $earthRadius = 3959)
    {
        // convert from degrees to radians
        $latFrom = deg2rad($latitudeFrom);
        $lonFrom = deg2rad($longitudeFrom);
        $latTo = deg2rad($latitudeTo);
        $lonTo = deg2rad($longitudeTo);
        $latDelta = $latTo - $latFrom;
        $lonDelta = $lonTo - $lonFrom;
        $angle = 2 * asin(sqrt(pow(sin($latDelta / 2), 2) +
                cos($latFrom) * cos($latTo) * pow(sin($lonDelta / 2), 2)));
        return $angle * $earthRadius;
    }

Some data:

array:3 [
  90210 => array:3 [
    "latitude" => "34.0900"
    "longitude" => "-118.4100"
    "hits" => 0
  ]
  90211 => array:3 [
    "latitude" => "34.0600"
    "longitude" => "-118.3800"
    "hits" => 0
  ]
  77389 => array:3 [
    "latitude" => "30.0600"
    "longitude" => "-95.3800"
    "hits" => 0
  ]
]

Answer 1

An alternative

   protected function haversineGreatCircleDistance($latitudeFrom, $longitudeFrom, $latitudeTo, $longitudeTo, $earthRadius = 3959)
    {
        // convert from degrees to radians
        $latFrom = deg2rad($latitudeFrom);
        $lonFrom = deg2rad($longitudeFrom);
        $latTo = deg2rad($latitudeTo);
        $lonTo = deg2rad($longitudeTo);

You can simplify this if you only call it from the one place.

    protected function haversineGreatCircleDistance($from, $to, $earthRadius = 3959)
    {
        // convert from degrees to radians
        $latFrom = deg2rad($from['latitude']);
        $lonFrom = deg2rad($from['longitude']);
        $latTo = deg2rad($to['latitude']);
        $lonTo = deg2rad($to['longitude']);

Not a big deal, but this is shorter.

Avoid unnecessary variables

                $validInputZips[$zip]['hits'] = 0;

You create this purely to use for a single iteration in a later loop. Why bother? Create a variable in the loop and let it die after. Or in this case, just take action in the loop and skip the variable altogether. More on that later.

Don't do many times what you can do once

If you do

          $diameter = 2 * $radius;

before the nested loops, you only have to do the multiplication once.

Keep it simple

            foreach ($maskedInputZips as $maskedZip => $maskedLatlon) {

                if ($zip != $maskedZip) {
                    $miles = $this->haversineGreatCircleDistance($latlon['latitude'], $latlon['longitude'], $maskedLatlon['latitude'], $maskedLatlon['longitude'], 3959);

                    if ($miles < ($radius * 2)) {
                        $validInputZips[$zip]['hits'] += 1;
                    }
                }
            }
            if ($validInputZips[$zip]['hits'] == 0){
                $outputArray[] = $zip;
            }

You can simplify this as

            foreach ($maskedInputZips as $maskedZip => $maskedLatlon) {

                if ($zip != $maskedZip) {
                    $miles = $this->haversineGreatCircleDistance($latlon, $maskedLatlon);

                    if ($miles < $diameter) {
                        continue 2;
                    }
                }
            }

            $outputArray[] = $zip;

You don't need to increment the variable, as you never use it except to check for 0. A simple Boolean is plenty. This skips to the next iteration of the outer loop as soon as it finds the zip that makes it unqualified. If that never happens, we add the zip to the output.

Careful about calling a function and explicitly entering the default value of a parameter. When you do that, it can make it confusing as to which value matters. The same value appears in two places, but only one matters. What if someone changes the wrong one?