Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring: High Level Overview #41

Open
p2 opened this issue May 11, 2015 · 4 comments
Open

Refactoring: High Level Overview #41

p2 opened this issue May 11, 2015 · 4 comments

Comments

@p2
Copy link
Member

p2 commented May 11, 2015

To make AppCore useable without needing to link the whole AppCore framework and with one's own AppDelegate, not a subclass of APCAppDelegate, some refactoring has to happen. Before I dig myself in I wanted to note a few ideas and see if it's a good way to go. I'll raise additional issues for specific things, hoping to collect some thoughts on the general refactoring direction here.

Untangle App Delegate

Ties to the specific implementation of APCAppDelegate is everywhere, often for sensible things like getting ahold of the data substrate instance, but not always. A possible way would be to create controller classes for the individual AppCore "modules", APCOnboarding and APCDataSubstrate are a start already. App delegates would have to implement a protocol so that they can return the controller instance to be used in the app. This would help modularizing and at the same time allow to clean up the initializationOptions dictionary.

The modules I currently see are:

  • Data Substrate (with the current user, see below)
  • Onboarding & Consenting
  • Scheduling
  • Data Collection

Untangle APCUser

The current implementation of the class representing the study subject, APCUser, is strongly tied to the Bridge SDK. In fact its sign-up and withdraw methods are only defined in the user's Bridge category. The current CoreData stack is likely to be of use to many ResearchKit developers, but still there will be studies with minimal on-device data that only need a name and the date of consent stored on device, possibly in the keychain.

I think the nicest way would be to convert APCDataSubstrate and APCUser into categories and make the current models implementations of those, for those who want to use them out of the box. The Bridge extended user should be a subclass of the current APCUser class, not being able to use methods not defined in the superclass/protocol.

@peculiar
Copy link
Member

These are excellent ideas.

Note: passive data collection for HealthKit and location has already been refactored and PR(s) should/will be pending. Likewise with scheduling and schedules.

p2 added a commit to chb/AppCore that referenced this issue May 11, 2015
@p2
Copy link
Member Author

p2 commented May 11, 2015

@peculiar Very good, and good to hear about the pending PRs. I'll be focusing on consenting for now.

@jwe-apple
Copy link
Member

@p2 Rather than relating the bridge user and the APCUser using subclassing, I'd rather see them use an object composition or peer relationship, i.e. one of them owns the other.

@p2
Copy link
Member Author

p2 commented May 12, 2015

@jwe-apple My immediate idea was to make the bridge user hold on to an object that mediates the backend exchange, hence a subclass of the basic APCUser to hold that particular relationship. I haven't looked to closely yet at what it takes to achieve the untangling, will open another task once I start and nobody beats me to it. I want to get a more general feel about the level of refactoring that everyone is comfortable with.

Erin-Mounts added a commit to Erin-Mounts/AppCore that referenced this issue Nov 17, 2015
Match pause button color and size to leave button
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants