Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed incorrect error descriptions in JSONArray transformer #240

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

dcaunt commented Feb 13, 2014

A couple of the error descriptions in the JSON Array transformer are incorrect.

I was looking at this in relation to #191 and how to bubble up errors and noticed that the errors thrown by the array transformer in attempting to transform each dictionary are the same as those the dictionary transformer would throw. See lines 180-192 and 114-123.

Shouldn't we just attempt the transformation in line 180 and returned the JSON dictionary transformer's error? Or we could provide more detail by returning the dictionary error via the NSUnderlyingErrorKey and detail which object in the array failed to transform.

@jspahrsummers jspahrsummers commented on the diff Feb 13, 2014

...SValueTransformer+MTLPredefinedTransformerAdditions.m
@@ -180,7 +180,7 @@ + (void)load {
if (![JSONDictionary isKindOfClass:NSDictionary.class]) {
if (error != NULL) {
NSDictionary *userInfo = @{
- NSLocalizedDescriptionKey: NSLocalizedString(@"Could not convert JSON array to model array", @""),
+ NSLocalizedDescriptionKey: NSLocalizedString(@"Could not convert JSON dictionary to model", @""),
@jspahrsummers

jspahrsummers Feb 13, 2014

Owner

I don't think this original description is actually incorrect, just a little ambiguous. How about something like:

Could not convert JSON array to model array because a JSON dictionary failed to parse

(But shorter, if possible.)

@robb

robb Feb 13, 2014

Owner

Embedded dictionary failed to parse

?

@jspahrsummers

jspahrsummers Feb 14, 2014

Owner

Dictionary within JSON array failed to parse

😉

Owner

jspahrsummers commented Feb 13, 2014

Shouldn't we just attempt the transformation in line 180 and returned the JSON dictionary transformer's error? Or we could provide more detail by returning the dictionary error via the NSUnderlyingErrorKey and detail which object in the array failed to transform.

It's the same error code either way, so I'm not sure that it'd matter much or add more detail to use the original NSError. If I had to pick, though, I'd say NSUnderlyingErrorKey would be the way to go.

Owner

robb commented Feb 13, 2014

NSUnderlyingErrorKey sounds good 👍

Member

dcaunt commented Feb 17, 2014

Great, I think it's a little clearer where the problem lies if we return an error with an NSUnderlyingErrorKey pointing to the dictionary error.

But where would that change leave #191 ? If the JSONArray transformer uses the proposed array mapping transformer, the top level error description will be less specific, as it would be something like Could not transform array rather than Could not convert JSON array to model array

Owner

jspahrsummers commented Feb 17, 2014

Honestly, I'm not super concerned about how specific the error message is, as long as:

  • It's traceable to a specific call (which is helped by Mantle being open source)
  • It includes the actual object or dictionary that couldn't be converted

@robb robb added the JSON label Mar 6, 2014

@robb robb added this to the 2.0 milestone Oct 30, 2014

Owner

robb commented Mar 4, 2015

I don't think this one is relevant anymore.

@robb robb closed this Mar 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment