search

getbouncer / cardscan-ios

A library for scanning credit and debit cards
MIT License
207 stars 66 forks source link

Add protocol to enable extending the main loop #136

Closed kingst closed 4 years ago

kingst commented 4 years ago

This PR is a first step to enabling people to reuse our UI and image pipeline while dropping in their own ML (ie main loop). Since there isn't an elegant way to subclass storyview-based VCs, having a protocol + public variables should expose enough to accomplish a number of jobs.

jaimejiyepark commented 4 years ago

Overall lgtm! I like the extensibility and how quickly you were able to spin up a prototype of the screen detection detector with this.

But I have some questions:

  1. In the blocking ml model, if it nil coalesces to true why would you ever return true in the main loop delegate function?

  2. Are we going to use this functionality to upgrade / update cardscan at all?

kingst commented 4 years ago

I have some answers :)

  1. it gives the caller the option to check stuff and run the base main loop conditionally. It's true that in my current use for it that function always returns false, but that flexibility seems useful. If you think we should change it to use the optional as a "all in" replacement for the main loop I'm open to it -- just let me know your preference.

  2. probably not, but it could make it easier to implement other types of challenges that involve scanning stuff. I was playing around with one such challenge and I'm going to use this mechanism for it. I can provide more details on slack (this is a public repo)

jaimejiyepark commented 4 years ago

  1. well the blocking ml model implementation is on our side so we can change it whenever so its not a big deal.

  2. gotcha!

shipit