-
-
Notifications
You must be signed in to change notification settings - Fork 732
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
Subclassing Record vs using a Struct #430
Comments
Hello @jfahrenkrug, You sum up quite well the available options. The best choice... depends on each situation, and also the taste of the developer. Back in the early GRDB days, we had no record protocols, but only the Record class. It was born out of my previous Objective-C experience around FMDB, and my pleasure using Ruby's Active Record. Time has passed, my Swift skills have improved, I learned about the benefits of structs, and Swift brought Codable to us. Eventually my own personal preference has turned to Codable structs + record protocols. Yet the Record class is still there. And it will stay. It is important because some people need a record type that is a class (for whatever reason). But the real reason why it is important is that some people expect to subclass a base class, especially people who come from FCModel, Core Data, Realm, Java, Kotlin, etc. I want those people to bring in their personal mental model of what a record type is, and feel welcomed. One doesn't need to learn about protocol-oriented design to use GRDB. And when one eventually learns about Swift structs, Codable, etc, new GRDB options turn available. I don't really know, currently, how to give a definitive advice. Even if I know quite well that sometimes people just want answers. Do you have your own opinion on this question? Now let's address some specifics of your question:
Record structs can't self-compare, because unlike Record subclasses, they don't have the storage that remembers their previous state. But you can still compare two record structs: let oldPlayer = try Player.fetchOne(db, key: 1)
var newPlayer = oldPlayer
newPlayer.score = 100
if try newPlayer.updateChanges(db, from: oldPlayer) {
print("player was modified, and updated in the database")
} else {
print("player was not modified")
}
Yes. Maybe Record could be enhanced so that it provides a better support for Codable. I don't really know, because my personal focus has turned to record structs: I haven't spent much time on the Record class recently. |
As you can see, there's a palatable tension between the claimed goal, and the reality 😅 Should it be addressed in the documentation, or in the code of the library? That's the question. |
As a support for reflexion, and an eventual new paragraph in the Record documentation, here is a quick tour of features and their current availability:
// Record subclass
class Player: Record { ... }
// Record protocols
struct Player: FetchableRecord, PersistableRecord { ... }
class Player: FetchableRecord, PersistableRecord { ... }
// Codable record
struct Player: Codable, FetchableRecord, PersistableRecord { ... }
class Player: Codable, FetchableRecord, PersistableRecord { ... } |
Hi @groue, Thank you so much for your quick and thorough reply! It's very helpful to understand the reasons behind the current state of things :) To answer your question:
I personally would like to do things in the most "Swifty" way. I've been an Objective-C guys for years and I'm getting more comfortable with Swift. I think the following line in the README might be a bit misleading:
It made me think that I'd get the most features if I subclass
I tend to think that (A) might be the better approach in the long run. That being said, it would be great to have a code example that shows how to do the Thank you so much for your help and this fantastic framework! Johannes |
+1 to this issue! I've been similarly confused on some of the direction. 1) I like the Table and it belongs in the docs someplace and 2) I like the push towards more swift standards that embrace the newer patterns away from the likes of Core Data and Realm. |
Thanks for your useful feedback! This discussion is a good way to gather our thoughts and make them clear 👍 And it's totally clear now that the doc needs an update. The fate of the Record class is not settled yet. Generally speaking, I like it when good practices are made possible. And I prefer opening doors than closing them. Unless I missed something, the Record class is not yet a problem which prevents the library from moving on. |
@groue Thank you, you are absolutely right. I seems as if it's primarily a documentation issue. The other half of the issue seems to be that |
I agree, this seems like a documentation only type of issue. It might be nice to have a breakdown on it's own about the record types. Don't get me wrong the documentation you have so far is great but it's also a little dense. I might propose a change to organize it a bit more for these types of questions users will have. |
I'm all ears. I appreciate a lot critics about documentation because I'm not a native English speaker, not a professional writer, and people usually don't comment on it at all. I wish there were a fast path inside GRDB records, so that one can quickly setup some working code without thinking much about it. From this starting point, users would then progressively discover the extra information and features that are relevant to their application. Even if the current documentation does not produce this effect, it was my humble goal ;-) Until now, I used to use the Record class as this entry point. I'm sure Codable records could become a lovely entry point. Maybe they should become this entry point. There are a few barriers on that way:
😅 |
OK maybe this is not a real barrier. After all, this is the code I write every day.
This one is really a problem. It has always been. I could never solve it. |
(Live thinking) If we are able to provide the fast path to records with protocols only, then the emphasis on the Record class would naturally vanish. After we get sure it is no longer useful at all, we could eventually sunset this class, stop documenting it, deprecate it, and remove it in a future major release. According to the above table, we would only lose the "self-comparison" feature. (we don't have to rush, or jump on conclusions) |
@groue I really like your suggestions. And above all: Your documentation is outstanding! It is very rare for an open source project to have such great documentation. To improve it, a "cookbook" section could be great too: "I get this kind of nested JSON from my API, how do I best consume, model, persist, and sync it with GRDB?" About |
Those three combinations can easily be found:
Those three others can be seen, too, when Codable code generation does not really help, or where the performance hit brought by Codable has to be avoided:
|
This is a tough documentation challenge. No matter how I try to put it, I get locked in a loop 😅. I need readers to know record features before they can choose which protocols they need. And I need the readers to know the protocols before they know the available features. I'll need several tries until I figure out how to address this quite valid issue. |
@groue Thanks so much for your work on this, and no rush :) |
Folks, I plan to use this very little change in the introduction to record types as a solution for this issue: f7b79e0. I don't know any straight answer to the question "Should I subclass Record or use a struct?" So I have been deliberately fuzzy and subjective: I hope that a choice between "swifty" and "classic" will match the targeted developer personalities and experiences 🤞😉 I could not add the above table in the doc, because I did not figure out how to make it useful. |
@groue Very nice! I think to address my original concern, it would be fantastic to mention in the docs that certain features are only available with the struct approach, like JSON columns etc. But it's up to you whether you want to add that. Thanks again! |
It's not only up to me :-) If you or another user feels like writing and helping other users make the best of record types, I and other GRDB collaborators will be glad reviewing a pull request. It's not only about code, as you see! |
Hello, if you have the time, I’m very curious about your opinion of this, particularly what’s said at the end. https://medium.com/commencis/stop-using-structs-e1be9a86376f I’m not sure if there’s something particular about the Record class that makes it less performant or if it’s that it uses a class over struct. My use is mainly a FetchedRecordsController for a table view. Currently not that large, less than 20,000 records, though that is perhaps a fair amount. Performance is already very fast though if there is some potential benefit to classes as suggested by that article — I am not so advanced in Swift to be able to analyze —, maybe it is worth switching. |
@gennaios GRDB is value/reference type agnostic. You can define records from structs, classes, or subclasses of Record. // OK
struct Player: FetchableRecord, PersistableRecord { ... }
// Sure
class Player: FetchableRecord, PersistableRecord { ... }
// Yeah
class Player: Record { ... } This means that the choice is eventually up to the application developers. Now I personally favor structs because I'm sure they won't mutate under my feet, and I can pass them across threads without thinking too much. This is so comforting. But this can not be an absolute advice: only you can take a decision about the lifetime of your records, instrument your app, perform experiments, and make a wise choice. You can also check the Performance page, if you want, even its goal is to compare the inner guts of several database libraries, and NOT to provide accurate numbers about realistic database uses. |
What did you do?
I'm new to the library. Thank you! I love it so far, but I'm having trouble deciding whether to make my models be
(a) subclasses of
Record
or (b) structs that implementCodable, FetchableRecord, MutablePersistableRecord
.The documentation points out that
Record
has change detection, which is something I'd like. On the other hand it seems as if thestruct
approach gives me JSON encoding for free, which I also like. When subclassing fromRecord
, it seems as if I need to do the JSON encoding and decoding by hand in theencode(to)
andinit(row:)
methods.What did you expect to happen?
It would be great if I'd get the same encoding/decoding luxury in
Record
subclasses. Maybe I'm just doing it wrong :-D.What happened instead?
It seems as if I get none of the encoding/decoding logic for free that I get when using structs.
There's very little documentation on how to use
Record
subclasses in advanced scenarios (like having JSON columns), so I might just be doing it wrong.Environment
GRDB flavor(s): GRDB
GRDB version: 3.4.0
Installation method: CocoaPods
Xcode version: 10.0
Swift version: 4.2
Platform(s) running GRDB: iOS
macOS version running Xcode: High Sierra
Demo Project
(none)
The text was updated successfully, but these errors were encountered: