Array from file

Question

I have this code which is performed each time I click the 'show polygons' button. The problem is that it takes a few seconds to finish running through the code before actually drawing the polygons + adding annotations to map. It actually takes about 10 seconds or so each time.

I am wondering if you can suggest any way to make my code 'more efficient' and speed it up.

firlist has about 605 lines of data entries and airportfilelines has almost 9000 lines of data entries.

- (void) updateATC {
    for (NSString *firline in firList) {

        NSArray *fields = [firline componentsSeparatedByString:@":"];

        for (int i = 0; i < atcOnline.count; i++) {

            ATC *atc = (ATC *)[atcOnline objectAtIndex:i];

             NSArray *callsign = [atc.callsign componentsSeparatedByString:@"_"];

            if ([[fields objectAtIndex:0] isEqualToString:[callsign objectAtIndex:0]] && ![atc.callsign containsString:@"_GND"] && ![atc.callsign containsString:@"_TWR"] && ![atc.callsign containsString:@"_DEL"] && ![atc.callsign containsString:@"_DEP"] && ![atc.callsign containsString:@"_APP"] && ![atc.callsign containsString:@"_ATIS"] && ![atc.callsign containsString:@"_SUP"] && ![atc.callsign containsString:@"OBS"]) {

               [ctrATC addObject:[atcOnline objectAtIndex:i]];

                for (NSString *line in firDisplayLines) {
                    if ([line hasPrefix:[NSString stringWithFormat:@"DISPLAY_LIST_%@",[fields objectAtIndex:5]]])

                    {
                        NSArray *sector = [line componentsSeparatedByString:@"+"];
                        CLLocationCoordinate2D coordinates[sector.count];

                        for (int i = 1; i < sector.count; i++) {

                            NSArray *coordinate = [sector[i] componentsSeparatedByString:@":"];

                            coordinates[i-1] = CLLocationCoordinate2DMake ([[coordinate objectAtIndex:0] doubleValue],[[coordinate objectAtIndex:1] doubleValue]);
                        }

                        MKPolygon *polygon = [MKPolygon polygonWithCoordinates:coordinates count:sector.count-1];

                        [firOverlays addObject:polygon];


                        dispatch_async(dispatch_get_main_queue(), ^{
                            [_mapView addAnnotations:ctrATC];
                            [_mapView addOverlays:firOverlays];
                        });


                    }
                }
            }

        }
    }

    for (NSString *line in airportFileLines) {

        NSArray *fields = [line componentsSeparatedByString:@":"];

        for (int i = 0; i < atcOnline.count; i++) {

            ATC *atc = (ATC *)[atcOnline objectAtIndex:i];

            NSArray *callsign = [atc.callsign componentsSeparatedByString:@"_"];


            if ([[fields objectAtIndex:0] isEqualToString: [callsign objectAtIndex:0]] && ![atc.callsign containsString:@"ATIS"] && ![atc.callsign containsString:@"SUP"] && ![atc.callsign containsString:@"OBS"] && ![atc.callsign containsString:@"CTR"] && ![atc.callsign hasPrefix:@"K"])
        {
            atc.coordinate = CLLocationCoordinate2DMake([[fields objectAtIndex:4] doubleValue],[[fields objectAtIndex:5] doubleValue]);

            [otherATC addObject:atc];

        } else if ([[[fields objectAtIndex:0] substringFromIndex:1] isEqualToString: [callsign objectAtIndex:0]] && [[fields objectAtIndex:3] isEqualToString:@"US"] && ![atc.callsign containsString:@"ATIS"] && ![atc.callsign containsString:@"SUP"] && ![atc.callsign containsString:@"OBS"] && ![atc.callsign containsString:@"CTR"]) {

            atc.coordinate = CLLocationCoordinate2DMake([[fields objectAtIndex:4] doubleValue],[[fields objectAtIndex:5] doubleValue]);

            [otherATC addObject:atc];
        }
        }
    }
   dispatch_async(dispatch_get_main_queue(), ^{
        [_mapView addAnnotations:otherATC];
    });
}

A sample line from firList:

EBBU:Brussels:BE:50.4:4.2:100

A sample line from airportFileLines:

KJFK:John F Kennedy:New York:US:40.64:-73.779

Answer 1

There are some style issues and other things that I think I'd like to point out, but perhaps that'll be better set aside for another answer.

For now, I mostly want to address the performance, and in particular, this comment:

No, I'm reading it from the entire array each time with a for loop. I read the file once on viewDidLoad.

There are probably ways that all these loops can be sped up some, but I think the bigger problem is there's a lot of work that's being unnecessarily repeated.

We should read the file once. After we've read the file once, we should part the file into strings (per line) which get parsed into their individual parts. These individual parts should be properties of an object. Perhaps they are, but you didn't include these classes. Some parts of your question are a little unclear (what some of these cryptically named classes actually are).

BUT... once you've done this once, there's no point in repeating all this work. At least not until you've reread the file.

If we haven't reread the file, then the data hasn't changed right? We just need to instantiate objects that represent the data when we read the file. Then we work with these objects.

All the work at this point still takes just as long the first time, but we've drastically improved the user experience by only performing this work when absolutely necessary.

---

One specific point where we can speed up how long this process actually takes is by concerting this classic for loop to a forin loop:

for (int i = 0; i < atcOnline.count; i++)

becomes...

for (ATC *atc in atcOnline)

We've already sped the code up by not needing to ask about the count every time. We also speed it up by completely eliminating this line:

ATC *atc = (ATC *)[atcOnline objectAtIndex:i];

And this line:

[ctrATC addObject:[atcOnline objectAtIndex:i]];

can become:

[ctrATC addObject:atc];

Answer 2

For each element from the first array firList you traverse the entire second array atcOnline to find a matching element.

Assuming that there is only one matching element for each "call sign", this can be much improved by creating a dictionary (a hash map) mapping the "call sign" to the corresponding element in the atcOnline array.

Moreover, there are a lot of "excluding conditions" which are always checked again in the inner loop. This can be improved by adding only those elements to the dictionary which are not to be excluded.

So this would look like

NSMutableDictionary *callsignToAtcMap = [NSMutableDictionary dictionary];
for (ATC *atc in atcOnline) {
    NSArray *callsign = [atc.callsign componentsSeparatedByString:@"_"];
    if (![atc.callsign containsString:@"_GND"] &&
        ![atc.callsign containsString:@"_TWR"] &&
        ![atc.callsign containsString:@"_DEL"] &&
        ![atc.callsign containsString:@"_DEP"] && 
        ![atc.callsign containsString:@"_APP"] && 
        ![atc.callsign containsString:@"_ATIS"] && 
        ![atc.callsign containsString:@"_SUP"] && 
        ![atc.callsign containsString:@"OBS"]) {
             callsignToAtcMap[[callsign objectAtIndex:0]] = atc;
    }
}

and has to be done only once if the contents of atcOnline does not change.

The two nested loops of your method do then reduce to one loop with a dictionary lookup, which should be faster:

- (void) updateATC {
    for (NSString *firline in firList) {
        NSArray *fields = [firline componentsSeparatedByString:@":"];
        ATC *atc = callsignToAtcMap[[fields objectAtIndex:0]] {
             ..
        }
    }
}