carekit-apple / CareKit

CareKit is an open source software framework for creating apps that help people better understand and manage their health.
https://www.researchandcare.org
Other
2.41k stars 444 forks source link

V2, the use of Codable, and entity identifiers #294

Closed jwaddilove closed 5 years ago

jwaddilove commented 5 years ago

This is feedback on editing and modifying care plans held in patient devices.

The sample app builds the doxylamine task thus:

let aFewDaysAgo = Calendar.current.date(byAdding: .day, value:  -4, to: thisMorning)!
let beforeBreakfast = Calendar.current.date(byAdding: .hour, value: 8, to: aFewDaysAgo)!
let afterLunch = Calendar.current.date(byAdding: .hour, value: 14, to: aFewDaysAgo)!

let schedule = OCKSchedule(composing: [
OCKScheduleElement(start: beforeBreakfast, end: nil,
interval: DateComponents(day: 1)),

OCKScheduleElement(start: afterLunch, end: nil,
interval: DateComponents(day: 2))
])

var doxylamine = OCKTask(identifier: "doxylamine", title: "Take Doxylamine",
carePlanID: nil, schedule: schedule)
doxylamine.instructions = "Take 25mg of doxylamine when you experience nausea."

It is easy to imagine that a patient could have other tasks in thier plan that have other medications which are scheduled beforeBreakfast and would use the same OCKScheduleElement or even the same OCKSchedule

When a task is serialised to JSON the schedule is embedded in the serialised item. If you consider two different tasks which share the same schedule being serialised and then de-serialised the result will be disconnected and duplicate schedules.

This is an issue for me as I build a care plan in an editor app and then send the serialised plan to the user's care app.

I'm wondering if it would be feasible to use the unique identifier to link the schedule, sort of: var doxylamine = OCKTask(identifier: "doxylamine", title: "Take Doxylamine", carePlanID: nil, scheduleNamed: "beforeBreakfastSchedule")

The same approach would be useful to link all the different entity types with the entire plan. This would simplify plan maintaince as fewer entities would need to be inculed in the plan edits.

I realise this a non-trival suggestion and I'm hoping there is an easier way of achiving the same effect. Thank you for taking the time to read this

erik-apple commented 5 years ago

I think what this boils down to is that schedules aren't designed to be shared between tasks. If you look back at the database slides from WWDC, you'll notice that the relationship from task to schedule element is one task to many schedule elements.

The intended behavior is that each task has its own schedule. Two tasks may have equal (in the value semantic sense) schedules, but cannot share one instance of a schedule (in a DB table row sense) because that would imply a many-to-many relationship between tasks and schedule elements. Maintaining the one-to-many relationship is advantageous for versioning of tasks.

Consider this scenario:

You create a taskA with sharedSchedule and then a taskB that also uses sharedSchedule. Sometime in the future, you update taskA by changing its schedule. This creates a new version of taskA that points to the new schedule. What should the expected behavior for taskB be? Does it still point to the old schedule? If so, then there was no reason for both tasks to share the schedule to begin with. If not, then creating a new version of taskA must implicitly create a new version of taskB.

Managing that complexity is possible, though non-trivial, and I'm not yet convinced it's an adventure we should embark on.

I am in favor of enabling your use case because I think it's an important one, but I think we may be able to find a value semantic compatible way to do it. Perhaps, for example, you could mark all tasks that share a beforeBreakfastSchedule by setting their groupIdentifier or tags property. When the app receives a notification from the server that the beforeBreakfastSchedule has been modified, it checks the tags or groupIdentifier on the new version of the schedule, queries for all tasks with a matching field, and updates each one in turn to the new schedule.

If you can tell us more about your setup, perhaps we can advise something appropriate for your particular arrangement.

One thing I would caution you against is using the localDatabseID of a schedule element to try to point two tasks at the same schedule element. If using the OCKStore, this will probably cause the second task to "steal" the schedule from the first and will probably cause an validation error when you try to persist the data.

jwaddilove commented 5 years ago

Erik,

Thank you for that very helpful answer. Now I understand schedules aren't designed to be shared between tasks I will re-think my design. I will digest your comments more fully and follow up later.

For the schedules, now I better understand your architecture, I can imagine providing template schedule elements in my plan editor which generate the task/schedule combination that works. I think I'm just guilty of thinking about an object graph that works for me but doesn't fit well with Core Data

erik-apple commented 5 years ago

If template schedules are a viable solution for your application, then that's definitely something you should consider. A number of our users took that approach with CareKit 1.2 and it worked out very well for them. It's a tried a true approach! 👍

On a side note, the object graph that you imagined would actually play with CoreData (the framework) very well. CoreData is built to let you think in object graphs instead of database tables, and it excels at that. The limitation is imposed not by CoreData, but by our OCKStore, and it is imposed in order to make it easier to reason about the versioning of tasks (like in the example I gave above). It's a trade off, but we believe it is more than worth it, because what we get in exchange is the ability to perform non-destructive updates and deletes, and keep a record of every version of a task that has ever existed!

erik-apple commented 5 years ago

@jwaddilove If you feel your question has been addressed, go ahead and close out this issue. If anything is still unclear, you're welcome to ask more questions too!

jwaddilove commented 5 years ago

Thank you