Only calculate fullName once, & sortable fullnames (#67)

1. Adds caching of the calculated fullName value
   (which will slightly improve performance)
2. Incorporates fullNames that respect the first-name-first
   rules of the currently unmerged PR #22
3. Adds two new fullName properties that can be used
   for sorting comparators
4. Move the comparator into the model object for easy testing

Includes tests to ensure that the first name first and last name
first sorts are behaving as expected.

// FREEBIE
pull/1/head
Russ Shanahan 9 years ago committed by Michael Kirk
parent 3083e2929c
commit 34ffce89f5

@ -43,6 +43,7 @@
B6273DDF1C13A2E500738558 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = B6273DDD1C13A2E500738558 /* Main.storyboard */; };
B6273DE11C13A2E500738558 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = B6273DE01C13A2E500738558 /* Assets.xcassets */; };
B6273DE41C13A2E500738558 /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = B6273DE21C13A2E500738558 /* LaunchScreen.storyboard */; };
D2AECE731DE8C3360068CE15 /* ContactSortingTest.m in Sources */ = {isa = PBXBuildFile; fileRef = D2AECE721DE8C3360068CE15 /* ContactSortingTest.m */; };
/* End PBXBuildFile section */
/* Begin PBXContainerItemProxy section */
@ -105,6 +106,7 @@
B6273DF01C13A2E500738558 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
B8362AB8E280E0F64352F08A /* libPods-TSKitiOSTestApp.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-TSKitiOSTestApp.a"; sourceTree = BUILT_PRODUCTS_DIR; };
C0DC1A83C39CBC09FB2405A3 /* libPods-TSKitiOSTestAppTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-TSKitiOSTestAppTests.a"; sourceTree = BUILT_PRODUCTS_DIR; };
D2AECE721DE8C3360068CE15 /* ContactSortingTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = ContactSortingTest.m; path = ../../../tests/Contacts/ContactSortingTest.m; sourceTree = "<group>"; };
D3737F7A041D7147015C02C2 /* Pods-TSKitiOSTestAppTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-TSKitiOSTestAppTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-TSKitiOSTestAppTests/Pods-TSKitiOSTestAppTests.release.xcconfig"; sourceTree = "<group>"; };
/* End PBXFileReference section */
@ -207,6 +209,7 @@
45B700961D9841E400269FFD /* OWSDisappearingMessagesConfigurationTest.m */,
4516E3E71DD153CC00DC4206 /* TSGroupThreadTest.m */,
4516E3E91DD1542300DC4206 /* TSContactThreadTest.m */,
D2AECE721DE8C3360068CE15 /* ContactSortingTest.m */,
);
name = Contacts;
sourceTree = "<group>";
@ -552,6 +555,7 @@
459850C11D22C6F2006FFEDB /* PhoneNumberTest.m in Sources */,
45458B7A1CC342B600A02153 /* TSStorageSignedPreKeyStore.m in Sources */,
453E1FDB1DA83EFB00DDD7B7 /* OWSFakeContactsUpdater.m in Sources */,
D2AECE731DE8C3360068CE15 /* ContactSortingTest.m in Sources */,
453E1FD81DA83E1000DDD7B7 /* OWSFakeContactsManager.m in Sources */,
454021ED1D960ABF00F2126D /* OWSDisappearingMessageFinderTest.m in Sources */,
4516E3EA1DD1542300DC4206 /* TSContactThreadTest.m in Sources */,

@ -12,12 +12,15 @@ NS_ASSUME_NONNULL_BEGIN
@class CNContact;
@class PhoneNumber;
@class UIImage;
@interface Contact : NSObject
@property (nullable, readonly, nonatomic) NSString *firstName;
@property (nullable, readonly, nonatomic) NSString *lastName;
@property (readonly, nonatomic) NSString *fullName;
@property (readonly, nonatomic) NSString *comparableNameFirstLast;
@property (readonly, nonatomic) NSString *comparableNameLastFirst;
@property (readonly, nonatomic) NSArray<PhoneNumber *> *parsedPhoneNumbers;
@property (readonly, nonatomic) NSArray<NSString *> *userTextPhoneNumbers;
@property (readonly, nonatomic) NSArray<NSString *> *emails;
@ -43,6 +46,8 @@ NS_ASSUME_NONNULL_BEGIN
#endif // TARGET_OS_IOS
+ (NSComparator)comparatorSortingNamesByFirstThenLast:(BOOL)firstNameOrdering;
@end
NS_ASSUME_NONNULL_END

@ -9,6 +9,10 @@ NS_ASSUME_NONNULL_BEGIN
@implementation Contact
@synthesize fullName = _fullName;
@synthesize comparableNameFirstLast = _comparableNameFirstLast;
@synthesize comparableNameLastFirst = _comparableNameLastFirst;
#if TARGET_OS_IOS
- (instancetype)initWithContactWithFirstName:(nullable NSString *)firstName
andLastName:(nullable NSString *)lastName
@ -91,17 +95,54 @@ NS_ASSUME_NONNULL_BEGIN
}
- (NSString *)fullName {
NSMutableString *fullName = [NSMutableString string];
if (self.firstName)
[fullName appendString:self.firstName];
if (self.lastName) {
[fullName appendString:[NSString stringWithFormat:@" %@", self.lastName]];
if (_fullName == nil) {
if (ABPersonGetCompositeNameFormat() == kABPersonCompositeNameFormatFirstNameFirst) {
_fullName = [self combineLeftName:_firstName withRightName:_lastName usingSeparator:@" "];
} else {
_fullName = [self combineLeftName:_lastName withRightName:_firstName usingSeparator:@" "];
}
}
return _fullName;
}
- (NSString *)comparableNameFirstLast {
if (_comparableNameFirstLast == nil) {
// Combine the two names with a tab separator, which has a lower ascii code than space, so that first names
// that contain a space ("Mary Jo\tCatlett") will sort after those that do not ("Mary\tOliver")
_comparableNameFirstLast = [self combineLeftName:_firstName withRightName:_lastName usingSeparator:@"\t"];
}
return _comparableNameFirstLast;
}
- (NSString *)comparableNameLastFirst {
if (_comparableNameLastFirst == nil) {
// Combine the two names with a tab separator, which has a lower ascii code than space, so that last names
// that contain a space ("Van Der Beek\tJames") will sort after those that do not ("Van\tJames")
_comparableNameLastFirst = [self combineLeftName:_lastName withRightName:_firstName usingSeparator:@"\t"];
}
return _comparableNameLastFirst;
}
- (NSString *)combineLeftName:(NSString *)leftName withRightName:(NSString *)rightName usingSeparator:(NSString *)separator {
const BOOL leftNameNonEmpty = (leftName.length > 0);
const BOOL rightNameNonEmpty = (rightName.length > 0);
if (leftNameNonEmpty && rightNameNonEmpty) {
return [NSString stringWithFormat:@"%@%@%@", leftName, separator, rightName];
} else if (leftNameNonEmpty) {
return [leftName copy];
} else if (rightNameNonEmpty) {
return [rightName copy];
} else {
return @"";
}
return fullName;
}
- (NSString *)description {
return [NSString stringWithFormat:@"%@ %@: %@", self.firstName, self.lastName, self.userTextPhoneNumbers];
return [NSString stringWithFormat:@"%@: %@", self.fullName, self.userTextPhoneNumbers];
}
- (BOOL)isSignalContact {
@ -124,6 +165,19 @@ NS_ASSUME_NONNULL_BEGIN
return identifiers;
}
+ (NSComparator)comparatorSortingNamesByFirstThenLast:(BOOL)firstNameOrdering {
return ^NSComparisonResult(id obj1, id obj2) {
Contact *contact1 = (Contact *)obj1;
Contact *contact2 = (Contact *)obj2;
if (firstNameOrdering) {
return [contact1.comparableNameFirstLast caseInsensitiveCompare:contact2.comparableNameFirstLast];
} else {
return [contact1.comparableNameLastFirst caseInsensitiveCompare:contact2.comparableNameLastFirst];
}
};
}
@end
NS_ASSUME_NONNULL_END

@ -0,0 +1,119 @@
// Created by Russ Shanahan on 11/25/16.
// Copyright © 2016 Open Whisper Systems. All rights reserved.
#import "Contact.h"
#import <XCTest/XCTest.h>
NS_ASSUME_NONNULL_BEGIN
@interface ContactSortingTest : XCTestCase
@end
@implementation ContactSortingTest
- (void)setUp
{
srandom((unsigned int)time(NULL));
}
- (void)testSortingNamesByFirstLast
{
NSComparator comparator = [Contact comparatorSortingNamesByFirstThenLast:YES];
NSArray<Contact *>*sortedContacts = [self.class contactArrayForNames:@[@[@"Adam", @"Smith"],
@[@"Adam", @"West"],
@[@"", @"Daisy"],
@[@"Daisy", @"Chain"],
@[@"Daisy", @"Duke"],
@[@"James", @"Smith"],
@[@"James", @"Van"],
@[@"James", @"Van Der Beek"],
@[@"Kevin", @"Smith"],
@[@"Mae", @"West"],
@[@"Mary", @"Oliver"],
@[@"Mary Jo", @"Catlett"],
]];
NSUInteger numContacts = sortedContacts.count;
for (NSUInteger i = 0; i < 20; i++) {
NSArray *shuffledContacts = [self.class shuffleArray:sortedContacts];
NSArray *resortedContacts = [shuffledContacts sortedArrayUsingComparator:comparator];
for (NSUInteger j = 0; j < numContacts; j++) {
Contact *a = sortedContacts[j];
Contact *b = resortedContacts[j];
BOOL correct = ([a.firstName isEqualToString:b.firstName] && [a.lastName isEqualToString:b.lastName]);
if (!correct) {
XCTAssert(@"Contacts failed to sort names by first, last");
break;
}
}
}
}
- (void)testSortingNamesByLastFirst
{
NSComparator comparator = [Contact comparatorSortingNamesByFirstThenLast:NO];
NSArray<Contact *>*sortedContacts = [self.class contactArrayForNames:@[@[@"Mary Jo", @"Catlett"],
@[@"Daisy", @"Chain"],
@[@"", @"Daisy"],
@[@"Daisy", @"Duke"],
@[@"Mary", @"Oliver"],
@[@"Adam", @"Smith"],
@[@"James", @"Smith"],
@[@"Kevin", @"Smith"],
@[@"James", @"Van"],
@[@"James", @"Van Der Beek"],
@[@"Adam", @"West"],
@[@"Mae", @"West"],
]];
NSUInteger numContacts = sortedContacts.count;
for (NSUInteger i = 0; i < 20; i++) {
NSArray *shuffledContacts = [self.class shuffleArray:sortedContacts];
NSArray *resortedContacts = [shuffledContacts sortedArrayUsingComparator:comparator];
for (NSUInteger j = 0; j < numContacts; j++) {
Contact *a = sortedContacts[j];
Contact *b = resortedContacts[j];
BOOL correct = ([a.firstName isEqualToString:b.firstName] && [a.lastName isEqualToString:b.lastName]);
if (!correct) {
XCTAssert(@"Contacts failed to sort names by last, first");
break;
}
}
}
}
+ (NSArray<Contact *> *)contactArrayForNames:(NSArray<NSArray<NSString *>*>*)namePairs
{
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
ABRecordID fakeRecordId = 0;
#pragma clang diagnostic pop
NSMutableArray<Contact *>*contacts = [[NSMutableArray alloc] initWithCapacity:namePairs.count];
for (NSArray<NSString *>*namePair in namePairs) {
Contact *c = [[Contact alloc] initWithContactWithFirstName:namePair[0]
andLastName:namePair[1]
andUserTextPhoneNumbers:@[]
andImage:nil
andContactID:fakeRecordId++];
[contacts addObject:c];
}
return [contacts copy]; // Return an immutable for good hygene
}
+ (NSArray*)shuffleArray:(NSArray *)array
{
NSMutableArray *shuffled = [[NSMutableArray alloc] initWithArray:array];
for(NSUInteger i = [array count]; i > 1; i--) {
NSUInteger j = arc4random_uniform((uint32_t)i);
[shuffled exchangeObjectAtIndex:(i - 1) withObjectAtIndex:j];
}
return [shuffled copy]; // Return an immutable for good hygene
}
@end
NS_ASSUME_NONNULL_END
Loading…
Cancel
Save