added a way to set custom key encoder/decoder #192
Conversation
|
🚫 CI failed with log |
Source/PINCache.h
Outdated
|
|
||
|
|
||
| /** | ||
| Multiple instances with the same name are allowed and can safely access |
There was a problem hiding this comment.
Yeesh, this is no longer true, do you mind changing the first sentence to:
Multiple instances with the same name are *not* allowed and can *not* safely access the same data on disk.
There was a problem hiding this comment.
I think the same needs to be done above to
Source/PINCache.m
Outdated
| @@ -39,7 +39,17 @@ - (instancetype)initWithName:(NSString *)name rootPath:(NSString *)rootPath file | |||
| return [self initWithName:name rootPath:rootPath serializer:nil deserializer:nil fileExtension:fileExtension]; | |||
| } | |||
|
|
|||
| - (instancetype)initWithName:(NSString *)name rootPath:(NSString *)rootPath serializer:(PINDiskCacheSerializerBlock)serializer deserializer:(PINDiskCacheDeserializerBlock)deserializer fileExtension:(NSString *)fileExtension | |||
| - (instancetype)initWithName:(NSString *)name rootPath:(NSString *)rootPath serializer:(PINDiskCacheSerializerBlock)serializer deserializer:(PINDiskCacheDeserializerBlock)deserializer fileExtension:(NSString *)fileExtension { | |||
| return [self initWithName:name rootPath:rootPath serializer:serializer deserializer:deserializer keyEncoder:NULL keyDecoder:NULL fileExtension:fileExtension]; | |||
There was a problem hiding this comment.
Nit: blocks are now objects so these should be nil, not NULL I believe?
Source/PINCache.m
Outdated
| rootPath:rootPath | ||
| serializer:serializer | ||
| deserializer:deserializer | ||
| keyEncoder:NULL |
Source/PINDiskCache.m
Outdated
| rootPath:rootPath | ||
| serializer:serializer | ||
| deserializer:deserializer | ||
| keyEncoder:NULL |
There was a problem hiding this comment.
nit: indentation and NULL instead of nil
Source/PINDiskCache.m
Outdated
| } | ||
|
|
||
| - (instancetype)initWithName:(NSString *)name | ||
| prefix:(NSString *)prefix | ||
| rootPath:(NSString *)rootPath | ||
| serializer:(PINDiskCacheSerializerBlock)serializer | ||
| deserializer:(PINDiskCacheDeserializerBlock)deserializer | ||
| keyEncoder:(PINDiskCacheKeyEncoderBlock)keyEncoder |
Source/PINDiskCache.m
Outdated
| @@ -129,6 +142,12 @@ - (instancetype)initWithName:(NSString *)name | |||
| return nil; | |||
| } | |||
|
|
|||
| if ((keyEncoder && !keyDecoder) || | |||
There was a problem hiding this comment.
Instead of throwing an exception, can you use NSAssert?
There was a problem hiding this comment.
I made like in serializer/deserializer case above. Should I change both to NSAssert ?
|
|
||
| //setup key encoder/decoder | ||
| if(keyEncoder) { | ||
| _keyEncoder = [keyEncoder copy]; |
There was a problem hiding this comment.
I don't think the copy is necessary anymore, but I'm ok with leaving it.
| { | ||
| return ^NSData*(id<NSCoding> object, NSString *key){ | ||
| return [NSKeyedArchiver archivedDataWithRootObject:object]; | ||
| }; | ||
| } | ||
|
|
||
| -(PINDiskCacheDeserializerBlock) defaultDeserializer | ||
| - (PINDiskCacheDeserializerBlock)defaultDeserializer |
There was a problem hiding this comment.
Thanks for the cleanup :)
|
@hovox Do you mind adding an entry to the CHANGELOG.md file? |
|
🚫 CI failed with log |
|
Should I remove |
…ception into NSAssert, changed NULL to nil, fixed indentions.
|
🚫 CI failed with log |
|
All tests are passed on my machine. |
|
@garrettmoon I have removed file extension, please let me know if it's okey. |
|
@hovox awesome, thank you for the PR! |
|
Thanks @garrettmoon for taking the time to let me know. This looks like a great PR and I have no issue moving to the more flexible solution using key encoders/decoders. |
|
Awesome, let's do this! |
Hi.
First, thanks for an amazing project. We are using ASDK and PINRemoteImage extensively in our app. We started to use PINCache as separate cache and there are cases when we want our custom filenames. We have added block for custom custom encoding/decoding, so anyone can provide their implementation for filenames. This can be used also for file extensions. Maybe in the next version extension can be replaced with custom blocks. Please let us know if it’s okey to merge.
Thanks.