groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.71k stars 689 forks source link

Throwing function calls should be more ergonomic #1615

Closed kdeda closed 1 week ago

kdeda commented 1 week ago

What did you do?

Wanted to write a custom init

    public init(
        _ db: Database,
        _ id: RecipeRecord.ID
    ) throws {
        let record = try RecipeRecord.fetchOne(db, id: id)
        self.init(record)
    }

Compiler not happy, went further and saw the fetchOne signature.

    public static func fetchOne(_ db: Database, id: ID) throws -> Self? {
        try filter(id: id).fetchOne(db)
    }

What did you expect to happen?

A throwing func should throw if anything goes wrong and return honest value in happy path. This will eliminate Optional types upstream. See code below.

    public init?(
        _ db: Database,
        _ id: RecipeRecord.ID
    ) throws {
        guard let record = try RecipeRecord.fetchOne(db, id: id)
        else { return nil }
        self.init(record)
    }

    func someFunc() {
            let dbRecipe: Recipe? = try await databaseClient.readTransaction { db in
                try RecipeRecord.fetchOne(db, id: recipe.id)
                if var rv = try Recipe.init(db, recipe.id) {
                    rv.fetchJoins(db)
                    return rv
                }
                return .none
            }
    }

If I wanted to convert a throwing func into an optional returning func I could just try?

        let record: RecipeRecord? = try? RecipeRecord.fetchOne(db, id: id)

I'd love to explain more and contribute on this excellent project.

What happened instead?

Needs improvement.

Environment

GRDB flavor(s): (GRDB, SQLCipher, Custom SQLite build?) GRDB version: Installation method: (CocoaPods, SPM, manual?) Xcode version: Swift version: Platform(s) running GRDB: (iOS, macOS, watchOS?) macOS version running Xcode:

Demo Project

groue commented 1 week ago

Hello @kdeda,

You are probably looking for find(_:id:)

public init(
    _ db: Database,
    _ id: RecipeRecord.ID
) throws {
    // Not an optional
    let record = try RecipeRecord.find(db, id: id)
    self.init(record)
}

All fetchOne methods in the library return an optional, which is nil when the fetched value does not exist in the database. fetchAll return an array, fetchSet a set, etc. This is very regular.

By constrast, find throws RecordError.recordNotFound if the record does not exist (i.e. if fetchOne would return nil).

kdeda commented 1 week ago

Excellent.

I guess you have reasons to support both ? fetchOne and find ?

groue commented 1 week ago

Sometimes the app logic must be ready for a record that does not exist: if let foo = try Foo.fetchOne(...). It is catching RecordError.recordNotFound that would be unergonomic.

At other times the app logic requires the record to be present, or can assume it is present due to some contextual information: foo = try Foo.find(...). It is dealing with an optional that would be unergonomic.

Pick the one you need.


For the little history, find is relatively recent (v6.5.0), and before that there was only fetchOne (first mention in v0.3.0).

The optional returned by fetchOne matches the SQLite behavior, where a SELECT statement that returns no row is totally ok (it is not an error).

But record types have specific needs, and eventually I really got fed up with those optionals: find came to life 😉

kdeda commented 1 week ago

find is what I was looking for. Thank you for the education.

So far this package has been fantastic. Thank you for it.

groue commented 1 week ago

Thank you! Happy GRDB!