-
Notifications
You must be signed in to change notification settings - Fork 0
Thumbnails and partial num search #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Once we actually switch to using this, the call to |
ios/RCTContacts/RCTContacts.m
Outdated
| [fetchRequest setUnifyResults:YES]; | ||
|
|
||
| NSError *regExError = nil; | ||
| NSRegularExpression *wordRegex = [NSRegularExpression regularExpressionWithPattern:@"\\w+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may want to check these errors and precreate them as part of the module init.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I'm understanding this correctly, variable provided as the error argument in NSRegularExpression is filled with an error object should one get thrown during the operation. Are you suggesting that I make this regular expression in the init and return early if an error gets created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is always the same, no reason to constantly create it.
ios/RCTContacts/RCTContacts.m
Outdated
| NSDictionary *contactDictionary = [self contactToDictionary:obj withThumbnails:NO]; | ||
| [contacts addObject:contactDictionary]; | ||
|
|
||
| NSArray *nameKeys = @[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These arrays can be pre-created since they are no longer a simple literal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you say pre-created, do you mean construct them in the init so they don't need to get recreated every time the method gets called?
I'll definitely change the mutable array into a vanilla NSArray though
|
I think this should run better. For some reason I couldn't directly declare the statics with literals. Also, I think it's crazy weird that |
tberman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this generally looks good, how much have you tested this?
The smaller change is that this exposes whether to include thumbnails as a boolean argument. This does end up changing the signature slightly
The search changes should emulate contact searching pretty closely. The partial number matching in contacts only searches the usual breakpoints from the beginning. For example, (123) 456 - 7890, a search for 56 will not match, but 45 will. This change just does full substring searching
I chose to primarily use vanilla for loops since https://stackoverflow.com/questions/992901/how-do-i-iterate-over-an-nsarray suggested that there was a speed benefit to doing it that way. I'm considering changing the outermost enumeration, but the speed seems pretty good as is