devedup / FlickrKit

An iOS Flickr Framework, written in Objective-C
http://www.devedup.com
219 stars 71 forks source link

Create photoUrl for original size photos correctly #18

Open jerbeers opened 10 years ago

jerbeers commented 10 years ago

See note on original size photos here: https://www.flickr.com/services/api/misc.urls.html

devedup commented 10 years ago

I've had a look at the code but I need to test this. I see on line 418 you default to jpg, but then you request the originalExtension from the photo dictionary on line 423 - which will only be there if it was added to the extras in the request. This means if you don't add it, then this line will return nil and you then have a nil extension.

Also, why even allow the user of to specify the extension as a param?... can an original photo be uploaded in multiple formats? If not, then why not either default to jpg, or look up what the actual extension is if its in the request dictionary and keep it simple?

What happens if I request the wrong extension?

jerbeers commented 10 years ago

The way I understand it, if you upload a photo of a different file format than jpg, Flickr will create all the other size photos as jpg, but the original is the original file format. The default of jpg is to keep the current state (see deleted line 441 that hard-codes jpg). Yes, you have to request the extras, but you could assert to validate that the keys are present. But even with that assumption, it's equal or better to what it does now, which is return a url for original size that doesn't work.

I added the extension parameter to allow the existing method signature to stay the same. Perhaps that method shouldn't be in the header but just an internal method. If you request the wrong extension, you'll get the same as you get now, a URL that returns a "not found" image.

Thanks for the review!

devedup commented 10 years ago

Ok that all makes sense. What I'll do is merge yours in (to give you credit where it's due) and then I'll make a few improvements on what you've started to make it a little more robust.

Thanks