This repository was archived by the owner on Jun 19, 2019. It is now read-only.
Refactor recording management #27
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Begins the separation of recording management and the dropbox syncing (JEFSyncingService)
JEFRecordingsController acts as a facade for both a repository and syncing service. This allows a single internal API that can be used for view controllers. Added a recordings provider protocol for cases where only read access is required.
There was a problem hiding this comment.
Curious why one NSObject * and one id?
There was a problem hiding this comment.
I need to call a KVO method at L43 but it's an informal protocol. I'll add a comment!
|
Tests pass and behaviour seems great in my testing :) |
|
@crayment Thanks for taking the time to look at this! Fixed your comments. |
|
Looks good 👍 |
interstateone
added a commit
that referenced
this pull request
Nov 14, 2014
Refactor recording management
interstateone
added a commit
that referenced
this pull request
Sep 26, 2017
Refactor recording management
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is some much-needed cleaning up and separation of how Jeff interacts with Dropbox.
The goals are to have less management code in the recordings VC and moved into proper controllers and model code. I'm anticipating the implementation of some of the Dropbox interaction changing further (see bottom). This is a step in that direction. This PR does incidentally fix the flickering table view issue.
I've structured this change like so: I treat the VC as part of the view layer, and now I have a facade controller it interacts with that owns a file repository and a file uploader. The file repository handles interacting with the file system. The file uploader handles uploading files, their progress and fetching public links for files. This separation is pretty blurry when using a service like Dropbox, but I think this should be a reasonable separation for using other services too, like, say, filesystem + Imgur. Regardless, it provides a reasonable level of code in each class. There are protocols for both of these classes that I try to think of first (instead of thinking of the classes themselves) as I find that better represents how those objects should communicate.
I also made a change that manipulates the list of recordings by placing them in a sorted index instead of inserting and then sorting the entire array. This was the cause of the aformentioned UI flickering as the table view tried to keep up.
In the future I'd like to: