Closed osteslag closed 6 years ago
Hello @osteslag,
Thanks for your compliments, and your question.
To be short, I have no direct recommendation.
As you may have noticed, GRDB records are very very close to raw database rows. If the subject of associations between records will make a humble entrance in the upcoming GRDB 3 (#313), this is a wide subject where best practices are still to be established.
Yet, I can give you hints, given your very clear sample code.
When I want to insert a new Album, I call save(_:) on it because it implements Persistable.
This sounds sensible.
extension Album: Persistable { func encode(to container: inout PersistenceContainer) { container["id"] = self.id container["title"] = self.title // I guess I can't do anything about the tracks here? They don't exist in the album table. }
You're right: the Album.encode(to:)
method only purpose is to fill columns for the album
table: you can't manage tracks here.
func insert(_ db: Database) throws { try self.performInsert(db) // Insert tracks here? I still don't have album.rowID. } func didInsert(with rowID: Int64, for column: String?) { self.id = Int(rowID) // Now I have rowID, but I can't throw or roll back in case something goes wrong when inserting the tracks. } }
Kudos for spotting performInsert
: this is the right method to call when you provide your own implementation for insert
. After performInsert
returns without error, didInsert(with:for:)
has been called: self.id
has been set to the newly inserted id.
You could thus make Track adopt Persistable, and write something like:
extension Track: Persistable {
func encode(to container: inout PersistenceContainer) { ... }
}
extension Album: Persistable {
func insert(_ db: Database) throws {
try performInsert(db)
// Insert tracks
for track in tracks {
track.albumId = id // track needs an albumId
try track.insert(db)
}
}
func didInsert(with rowID: Int64, for column: String?) {
self.id = Int(rowID)
}
}
This should work.
Now... This leaves many questions without answer, so I'm not sure this could eventually be part of a set of recommended best practices:
If insert is implemented, what about delete/update? Shouldn't they all have to synchronize the content of the database with the content of the Album.tracks
property?
If you want to make the Album class responsible for its tracks, then how do you make sure that all Album instances always have the correct tracks (when fetching/updating/deleting)? Is is even easy to define what "correct" is? This may turn tricky.
I want to make you understand that a tracks
property in the Album
class, which is used in the customized Album.insert
method, makes the Album class responsible for its tracks, in memory, and in the database. This is the principle of single responsibility in action.
Unfortunately, it looks to me that a plain tracks
property is far from enough to make the Album class able to handle this responsability fully, and correctly.
I'll describe as shortly as possible my own practices:
Record types are only responsible for their table, nothing else. The Album type has no tracks
property. Track type has an albumId
property, but no album
property. They barely wrap database rows.
Graphs of records are handled by specific types. Usually, at the beginning of application development, this is performed independently by each controllers, view model, this kind of object (YMMV). Sometimes you need albums without their tracks. Sometimes you need a (track, album) pair and ignore other tracks. Sometimes you need a full album info. Plain records can adapt to all these situations, so that controllers can do their job without much caring about the rest of the world.
After the needs of your application regarding album management become clear, it may become useful to eventually design a reusable type that would be fully responsible for an album and its tracks (or all albums and their tracks, or whatever relevant). But it is highly unlikely that this object can be designed from scratch, even before you know the full set of operations that need to be performed on albums and tracks.
In summary, my own experience tells me that dealing with the data my apps need, at the moment it needs it, provides a simple enough development model that can gradually and organically expand. Simple things remain simple, complex things come at their time. As long as the database schema is clean, simple and complex things can coexist in the application code.
Yet I will certainly not prevent you from building your own experience. It may be possible to have the Album record able to fully manage its tracks, in a way that respects all safety belts needed by applications (data consistency, thread safety, etc.)
Thanks for your elaborate answer, @groue.
After
performInsert
returns without error,didInsert(with:for:)
has been called:self.id
has been set to the newly inserted id.
I had missed that. At least it makes my approach possible. As you write, Track
would need an albumId
. Or one could make a bridging type, wrapping this id and the original track. But I do see it as a dead-end. For completeness, this is how an implementation could look:
extension Album: Persistable {
static var databaseTableName: String { return "album" }
func encode(to container: inout PersistenceContainer) {
container["id"] = self.id
container["title"] = self.title
}
func insert(_ db: Database) throws {
try self.performInsert(db)
// Contrary to my original understanding, we now have the rowID in self.id, but Track doesn't have an albumId we can store it in for when calling insert(_:) below. As a workaround, we can make a private bridging type. This doesn't scale well.
guard let albumId = self.id else { fatalError() }
for track in self.tracks {
let dbTrack = DBTrack(albumId: albumId, wrapped: track)
try dbTrack.insert(db)
}
}
func didInsert(with rowID: Int64, for column: String?) {
self.id = Int(rowID)
}
}
fileprivate extension Album {
/// No need to make it `MutablePersistable` because we're never updating it with any `rowID`. We could make a `Wrapped` protocol to streamline this approach, but I'm already not liking it.
fileprivate struct DBTrack: Persistable {
let albumId: Int
let wrapped: Track
static var databaseTableName: String { return "track" }
func encode(to container: inout PersistenceContainer) {
container["number"] = self.wrapped.number
container["title"] = self.wrapped.title
}
}
}
And you’re right, we haven’t even dealt with updates and deletions. I appreciate your thoughts, including those on Graphs of records, and see the value in having separate logic handle the relations between database tables.
Curiously, I ended up going in a whole other direction which has resulted in a flat structure. But this discussion have made me think of my app’s data persistency in a better way. Thank you.
Feel free to close the issue.
You're welcome @osteslag! Between raw SQLite/FMDB/SQLite.swift on one side, and Core Data/Realm on the other side, sits a strange outsider: GRDB. Not managed, and thus highly focused on as clear as possible concurrency guarantees; immutability-friendly, and thus not focused at all on graph mutations; record-driven, but surprisingly dry and limited when it comes to dealing with relations, because strict data correctness is favored over loosely-defined convenience features. We still have to invent our ways to use it - and I actually don't expect there is a single One True Way. I'll be happy hearing from you when you come up with a solution that works well for you: when such questions pop out, I'm happy to share my own experience, but I'm not the alpha and omega of application design ;-)
A new guide has shipped in GRDB 3: Good Practices for Designing Record Types.
Thanks for a well-opinionated Good Practices document. Very helpful in the process of designing data model and types. 👍
Thank you @osteslag! It's good to know that those guides are helpful 👍
First I have to say this is one of the most well-designed and documented toolkits I’ve come across in a long while. Thank you so much for putting in the effort.
I’m probably therefore also overlooking the obvious, but as a newbie I have a hard time finding out a good way of inserting related/referenced records in a one-to-many relation. For simplicity, let’s look at an
Album
and itsTrack
elements:Let’s say the Swift types look like this:
When I want to insert a new
Album
, I callsave(_:)
on it because it implementsPersistable
. The comments in the code below illustrate my troubles when I want to insert the tracks into thetrack
table.What am I missing here? How should I set it up so that when saving the album, all the tracks are saved as well?