-
Notifications
You must be signed in to change notification settings - Fork 297
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
CBL-5181: Array Index API tests and adjustments #3330
Conversation
@@ -33,7 +35,7 @@ | |||
|
|||
NS_ASSUME_NONNULL_BEGIN | |||
|
|||
@interface CBLCollection () <CBLRemovableListenerToken> | |||
@interface CBLCollection () <CBLRemovableListenerToken, CBLIndexableInternal> |
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 don't see a reason to create a protocol for CBLIndexableInternal as CBLCollection will not be used as CBLIndexableInternal type. I think you can just add a method indexesInfo
in CBLCollection() directly.
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
@protocol CBLIndexableInternal <CBLIndexable> |
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.
Don't need to create the protocol for this as the method can be put directly to CBLCollection Internal.
Also no need to have underscored prefix as this function will not be a public API.
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.
One comment and the rest is looking good.
[NSException raise: NSInvalidArgumentException format: | ||
@"Expressions cannot be empty "]; | ||
@"Empty expressions is not allowed, use nil instead"]; |
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.
Can you add another validation for a single empty string ? We don't allow that to be set as well. See the updated API spec.
Internal
section at the end of file forObjective-C/CBLCollection.mm
indexesInfo
which is exactlycoll.indexes()
but keeping all information for each index found, not only stripping the name out of itexpectExcepion
typo