Alua-Kinzhebayeva / iOS-PDF-Reader

PDF Reader for iOS written in Swift
MIT License
533 stars 150 forks source link

Protocol Support for PDFDocument #34

Closed benbahrenburg closed 7 years ago

benbahrenburg commented 7 years ago

@ranunez after thinking about it alittle more I think there is a better approach then the one I first suggested. If a protocol is used for the PDFDocument it would allow those using the project to create their own "document" as long as they conform to the interface.

For those using the existing PDFDocument there is no change as PDFDocument inherits from the protocol. I did have to make fileURL and fileName optional as someone calling using Data or CGPDFDocument wouldn't have these.

If you are ok with a protocol approach like this let me know and I'll create documentation and examples as part of the PR process.

You can see the very minor changes needed to support this approach in the below branch https://github.com/benbahrenburg/iOS-PDF-Reader/tree/exp2

Let me know if you'd like to discuss further.

ranunez commented 7 years ago

This seems like a much cleaner solution, but I'm still weary of making fileURL optional, as several features of the library depend on them, for example the print and activity sheet. If this were to be optional, how would we handle these buttons/actions, and how would this get communicated to the user of the library, especially at the point of initialization?

benbahrenburg commented 7 years ago

I guess I am looking at it from a flexibility standpoint. It would allow consumers to use the project with a more flexible set of PDF sources, similar to vfr reader supports. We could handle the button behavior via documentation, examples, and asserts.

Happy to put together all of the above so it is clear to the developers using the project.

ranunez commented 7 years ago

My main concern is making it clear to users what features/customizations are available when. If we can add the flexibility of a protocol oriented approach and not compromise on clarity and ease-of-use for developers using the project, I think we're golden. I'd be happy to take a look at a pull request.

ranunez commented 7 years ago

Closing due to inactivity. Free free to re-open if you want to open up a pull request for this in the future.