patternMinor
Array from file
Viewed 0 times
arrayfromfile
Problem
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.
```
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]);
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]);
Solution
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
becomes...
We've already sped the code up by not needing to ask about the
And this line:
can become:
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];Code Snippets
for (int i = 0; i < atcOnline.count; i++)for (ATC *atc in atcOnline)ATC *atc = (ATC *)[atcOnline objectAtIndex:i];[ctrATC addObject:[atcOnline objectAtIndex:i]];[ctrATC addObject:atc];Context
StackExchange Code Review Q#62211, answer score: 5
Revisions (0)
No revisions yet.