Skip to content
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

Made automatic NSURL transfom to work even if JSONTransformerForKey method defined #685

Merged
merged 3 commits into from
Mar 17, 2016

Conversation

k06a
Copy link
Contributor

@k06a k06a commented Mar 10, 2016

This works nice:

@interface Video
@property (strong, nonatomic) NSURL *streamUrl;
@property (strong, nonatomic) Money *cost;
@end

@implementation Video
+ (NSValueTransformer *)costJSONTransformer {
    return [MTLValueTransformer transformerUsingForwardBlock:^id(id value, BOOL *success, NSError *__autoreleasing *error) {
        return [MTLJSONAdapter modelOfClass:[Money class] fromJSONDictionary:value error:nil];
    }];
}
@end

But this gives and NSURL transform error:

#import <libextobjc/extobjc.h>

@interface Video
@property (strong, nonatomic) NSURL *streamUrl;
@property (strong, nonatomic) Money *cost;
@end

@implementation Video
+ (NSValueTransformer *)JSONTransformerForKey:(NSString *)key {
    MTLValueTransformer *costTransformer = [MTLValueTransformer transformerUsingForwardBlock:^id(id value, BOOL *success, NSError *__autoreleasing *error) {
        return [MTLJSONAdapter modelOfClass:[Money class] fromJSONDictionary:value error:nil];
    }];

    return @{
        @keypath(Video.new, cost): costTransformer,
    }[key];
}
@end

Solution is to add this line:

@keypath(Video.new, streamUrl): [NSValueTransformer valueTransformerForName:MTLURLValueTransformerName],

But I really like automatic NSURL/BOOL transforming. Why it should not work when method JSONTransformerForKey: is defined?

@robb
Copy link
Member

robb commented Mar 11, 2016

Thanks for the pull request, could you add some tests to that?

if (transformer != nil) {
result[key] = transformer;
continue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use tabs for indentation.

@k06a
Copy link
Contributor Author

k06a commented Mar 11, 2016

@robb heh, I'm new to Quick and Nimble test frameworks. Is it possible to test without new class declaration?

@robb
Copy link
Member

robb commented Mar 11, 2016

Probably not, but be have a bunch of small test classes already that should offer a good starting point.

@k06a k06a changed the title Made automatic NRURL transfom to work even if JSONTransformerForKey method defined Made automatic NSURL transfom to work even if JSONTransformerForKey method defined Mar 11, 2016
@k06a
Copy link
Contributor Author

k06a commented Mar 14, 2016

@robb I've added test case, that fails on the old version and passes on the new :)

@ikesyo ikesyo mentioned this pull request Mar 17, 2016
robb added a commit that referenced this pull request Mar 17, 2016
…ways

Made automatic NSURL transfom to work even if JSONTransformerForKey method defined
@robb robb merged commit cdb82df into Mantle:master Mar 17, 2016
@robb
Copy link
Member

robb commented Mar 17, 2016

Thanks ✌

@k06a k06a deleted the fix/free-nsurl-transform-to-work-always branch March 17, 2016 21:14
@k06a
Copy link
Contributor Author

k06a commented Mar 17, 2016

@robb, do you have any plans about next (2.0.6 or 2.1.0) release date?
Can't wait to throw out a bunch of NSURL transformers from my projects :)

@robb
Copy link
Member

robb commented Apr 15, 2016

@dcaunt as Release-Cutter in Chief, would you mind cutting a point-release? 🙇

@dcaunt
Copy link
Member

dcaunt commented Apr 15, 2016

I've released 2.0.7 to avoid any confusion about 2.0.6.

@robb
Copy link
Member

robb commented Apr 15, 2016

@dcaunt Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants