CodeSOD: The Bogus Animation

Animations have become such an omnipresent part of our UI designs anymore that we tend to only notice them when they're bad.

Ben is working on an iOS application which appeared to have a "bad" animation. In this case, it's bad because it's slow. How slow? Well, they have a table view with ten items in it, and the items should quickly tween to their new state- position, text, colors all could change in this process. And it was taking four seconds.

Four seconds to update ten items is a lot. Now, their application does have a lot of animations, and the first suspicion was that there was some ugly interaction between animations that was causing it to take a long time. But upon digging in, Ben discovered it wasn't the animations at all.

- (NSArray<NSString *> *)_combineTitles:(NSArray<NSString *> *)oldTitles with:(NSArray<NSString *> *)newTitles{ NSMutableSet<NSString *> *mergedSet = [NSMutableSet setWithArray:oldTitles]; [mergedSet unionSet:[NSSet setWithArray:newTitles]]; NSMutableArray<NSString *> *combinedTitles = [mergedSet.allObjects mutableCopy]; // TODO - this is a terrible method! // We should be able to properly determine the/a correct order of combinedTitles, not // simply trying random orders until we find the right one // Note unless the assumption stated in _reloadTableDataAnimatedAdvanced is met, this could loop forever // A better way to do this would be to consider the diff between oldTitles and newTitles // E.g. the diff for ABC -> CDE has insertions at 1, 2 and deletions at 0, 1 and therefore the diff is based // on the deletions being performed first - instead we want to construct the intermediate ABCDE which has insertions at 3, 4 while (YES) { DiffUpdate *diffUpdate1 = [DifferUtils getDiffWithOldData:oldTitles newData:combinedTitles]; DiffUpdate *diffUpdate2 = [DifferUtils getDiffWithOldData:combinedTitles newData:newTitles]; if (diffUpdate1.moves.count == 0 && diffUpdate2.moves.count == 0) { return [NSArray arrayWithArray:combinedTitles]; } // Fisher-Yates shuffle // https://stackoverflow.com/a/33840745 for (NSUInteger i = combinedTitles.count; i > 1; i--) { [combinedTitles exchangeObjectAtIndex:i - 1 withObjectAtIndex:arc4random_uniform((u_int32_t)i)]; } } return nil;}

Obj-C is a bit odd to read, but this defines a method that combines a list of old item titles with a list of new item titles. The resulting output should be sorted based on the insertion order of the items. And how do we do this in this code?

Well, we check if the output list is in the right order by taking a diff between the output and the two inputs. If it's in the right order, great, return our results. If it's not" we do a Fisher-Yates shuffle, which is to say, this is an actual bogosort in the wild.

At least the documentation is useful. Not only does it include the accurate This is a terrible method, but goes on to lay out exactly what a better method might look like.

That's not the best part of the comment, though. It's this one:

// Note unless the assumption stated in _reloadTableDataAnimatedAdvanced is met, this could loop forever

[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.