if properties[@"image_url"] is NSNull, then EVStory will crash

[seansu4you87]

EVStory.m line 246

Trying to make a NSURL out of an NSNull will crash execution. This brings up the larger problem of us having a brittleness in our parsing code. We need to come up with a strategy that makes our data parsing more robust.

I recommend a strategy similar to Mantle. They created a mapping strategy where one can go from NSObject property to NSDictionary key=>value. A 'transformer' can be provided which automatically maps to common types such as NSURL. The code path is the same, so one only needs to write type safety checks for NSNull, etc once. I found their interface slightly confusing, but that might be because it's very powerful. However, the mapping idea is a good one, and if we had that functionality, we can significantly reduce the number of crashes we have due to Vine-API incompatibilities.

Lemme know what you think!

PS
Using an Issue on Github, because I believe you can make commits with issue numbers, and directly create Pulls out of them, which is pretty cool. Just wanted to try it out

[seansu4you87]

@jhankin @justinbrunet comment here when you have a chance, I want to know what you guys think

[justinbrunet]

I absolutely agree our parsing needs to be more solid; it shouldn't be crashing under any circumstances. We should a) be testing it, but also b) just have something that is less likely to crash and burn. However, I thought Mantle was borderline un-readable, so I did a quick implementation of what I thought was the general idea of it (code referenced above).

Maybe the confusing-ness is because their interface is more powerful, but I think something along the lines of what I did could give us close to what we need without the heavy-handedness of Mantle.

[jhankin]

I agree with all of the above. I also think Mantle is kind of ugly, but at the same time, I'm fascinated by the idea of the NSValueTransformer. We could have generalized instances/subclasses of NSValueTransformer to convert certain common things (numbers, amounts, what have you) both back and forth, which would (theoretically) give us the ability to have setProperties: and dictionaryRepresentation be more or less automatic and deterministic.

Justin, I think your code makes sense but I'm not that crazy about the idea of doing voodoo with mapping selectors and whatnot -- especially because it seems like each property needs its own mapping method, rather than being able to factor out commonalities (like unconfirmedMappingForValue and facebookConnectedMappingForValue). I'm down with doing our own implementation of something Mantle-like, though, rather than just throwing the whole Mantle kitchen sink in there.

[justinbrunet]

Isn't Mantle doing the exact same thing? They have a URLJSONTransformer method, a HTMLURLJSONTransformer method, a stateJSONTransformer method, etc for every property. The only difference is the ones that are making a URL, for example, then call the same method within the method for each property, which would be easy enough to factor out.

As for mapping selectors voodoo, if you dig deeper into Mantle, it begins playing with the objective-c runtime to do the same voodoo. It's just hidden a few classes up (and the voodoo part here should be too, I was just trying to make it obvious what it was doing).

[justinbrunet]

Maybe we should just try throwing Mantle in later and see how it plays out of the box with our setProperties methods and subclassing chains.

[jhankin]

Fair point. I guess any time we're going to be making some kind of automagic parsing code, there's gonna be some voodoo involved. It does make sense to try throwing Mantle in and see how things go before we spend a lot of our (precious) time reinventing the wheel.

[seansu4you87]

1. Awesome job, overall that looks a lot cleaner than Mantle. Is there anyway where we can infer the type of a property (ie we get an @"avatarURL" string, find the property on the object, and figure out that it is of type NSURL), and then provide the @"avatarURL": @"avatar_url" mapping a default URLTransformer? This way, we can configure our parser with default Transformers for strings, floats, urls, bools, etc, and then also add in Application specific ones such as MoneyTransformers. Even more complex, we could potentially create UserTransformers, so that anytime a property of type EVUser is mapped to an appropriate JSON dictionary, we can automagically convert!

Taking it another level, a Mapping itself, is almost enough to be a Transformer, i.e. when I write a Mapping for an EVUser, it itself can automatically be used as a UserTransformer for an EVPayment's mapping. The only thing we would need to add are edge case functions for when shit is nil or NSNull

1. Ya, if Mantle works, then why not lol. I do agree with ya'll that it seems more confusing than need be. Not surprising given that the guy who wrote Mantle also wrote ReactiveCocoa

[justinbrunet]

I think it is possible to determine the type of a property, but I also think it's pretty complex. Not entirely sure, though, would need to mess around.

[jhankin]

It's definitely not pretty. See http://stackoverflow.com/questions/769319/how-to-detect-a-property-return-type-in-objective-c for details.

As an example, I put the following into [EVObject setProperties:] to see what would happen:

unsigned int outCount;
objc_property_t *propertyList = class_copyPropertyList([self class], &outCount);
for (int i = 0; i < outCount; i++) {
    objc_property_t property = propertyList[i];
    const char *name = property_getName(property);
    const char *attr = property_getAttributes(property);
   NSLog(@"Name: %s  Attributes: %s", name, attr);
}

And what came out looked sort of like this:

2013-08-21 16:22:53.439 Evenly[14027:3d03] -[EVObject setProperties:] [Line 178] PROPERTY DESCRIPTION OF <0xd0458c0> User 27: Colby Leachman (balance: 0)
2013-08-21 16:22:53.439 Evenly[14027:3d03] Name: name  Attributes: T@"NSString",&,N,V_name
2013-08-21 16:22:53.439 Evenly[14027:3d03] Name: email  Attributes: T@"NSString",&,N,V_email
2013-08-21 16:22:53.439 Evenly[14027:3d03] Name: phoneNumber  Attributes: T@"NSString",&,N,V_phoneNumber
2013-08-21 16:22:53.439 Evenly[14027:3d03] Name: password  Attributes: T@"NSString",&,N,V_password
2013-08-21 16:22:53.439 Evenly[14027:3d03] Name: balance  Attributes: T@"NSDecimalNumber",&,N,V_balance
2013-08-21 16:22:53.439 Evenly[14027:3d03] Name: updatedAvatar  Attributes: T@"UIImage",&,N,V_updatedAvatar
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: privacySetting  Attributes: Ti,N
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: rewardSharingSetting  Attributes: Tc,N
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: connections  Attributes: T@"NSArray",&,N,V_connections
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: currentPassword  Attributes: T@"NSString",&,N,V_currentPassword
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: facebookConnected  Attributes: Tc,N,V_facebookConnected
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: roles  Attributes: T@"NSArray",&,N,V_roles
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: needsGettingStartedHelp  Attributes: Tc,R,N
2013-08-21 16:22:53.440 Evenly[14027:3d03] Name: needsPaymentHelp  Attributes: Tc,R,N
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: needsRequestHelp  Attributes: Tc,R,N
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: needsDepositHelp  Attributes: Tc,R,N
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: unconfirmed  Attributes: Tc,N,GisUnconfirmed,V_unconfirmed
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: hasAddedCard  Attributes: Tc,R,N
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: hasAddedBank  Attributes: Tc,R,N
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: hasSentPayment  Attributes: Tc,R,N
2013-08-21 16:22:53.441 Evenly[14027:3d03] Name: hasSentRequest  Attributes: Tc,R,N
2013-08-21 16:22:53.442 Evenly[14027:3d03] Name: hasInvitedFriends  Attributes: Tc,R,N
2013-08-21 16:22:53.442 Evenly[14027:3d03] Name: avatar  Attributes: T@"UIImage",&,N,Vavatar
2013-08-21 16:22:53.442 Evenly[14027:3d03] Name: avatarURL  Attributes: T@"NSURL",&,N,VavatarURL

[justinbrunet]

Well, it could be worse I suppose.

[seansu4you87]

That doesn't seem too bad. Just get the first element in attr, call NSClassFromString on it, and then go find the Transformer for that Class

[jhankin]

Plus parse for the encoding types of the non-object ones. All the BOOLs
come up as "c", for example.

On Wed, Aug 21, 2013 at 4:51 PM, Sean Yu [email protected] wrote:

That doesn't seem too bad. Just get the first element in attr, call
NSClassFromString on it, and then go find the Transformer for that Class

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/247#issuecomment-23059488
.

Joe Hankin
410.371.1162
http://joehankin.com