SQLite.swift icon indicating copy to clipboard operation
SQLite.swift copied to clipboard

I wonder if this library could be written even safer.

Open Evertt opened this issue 8 years ago • 2 comments

One thing I love about this library is that you don't reference columns by string literals, but instead by variables that hold expressions. Like here:

let name = Expression<String?>("name")
users.insert(name <- "Alice")

And here:

for user in try db.prepare(users) {
    print(user[name])
}

However, these expressions are not necessarily tied to the table that you are using them on. You are just initializing them yourself and then using them on the table. There is no guarantee that the users table actually has a column named "name". One could also easily make a typo when writing the string literal "name".

So I was wondering, what if you made a new struct called Column or something, which has a private initializer, so only the library can initialize it. It would work a bit like this:

guard let name = users.columns["name"] else {
    throw AnError("the name column does not exist")
}

users.insert(name <- "Alice")
let user = users.first!
print(user[name])

You see this way we can at least 90% guarantee that the name column is actually a valid column for the users table. It's still not 100% of course, because a developer could still write the following and the compiler would not complain:

guard let name = users.columns["name"] else {
    throw AnError("the name column does not exist")
}

posts.insert(name <- "Alice") // note the incorrect table

However, I reckon this situation is far less likely than someone making a mistake in the current system.

I'd even want to go one step further and apply the same to tables. So right now you can just initialize a table (let users = Table("users")) and start using it, even if there is no such table in the database. You can make that much safer by doing something like this:

let users: Table

if let existingUsersTable = db.tables["users"] {
    users = existingUsersTable
} else {
    users = db.createTable("users") {
        // haven't thought this part out yet...
    }
}

What do you guys think? Of course this would be quite a big overhaul so this could only be implemented in the next major version if you actually like this change.

Evertt avatar Dec 21 '16 15:12 Evertt

Oh I just thought of a way how to make it even safer, like 100% safe. If you make it so that tables are not objects, but tables are classes, then you can enforce that columns should be of the right type. Here's a almost-proof-of-concept you can put in a playground.

This goes in the sources:

// Blabla.playground/Sources/main.swift

// This protocol is here, because we need to have
// a type-erased version of the PrimitiveDataType protocol
public protocol DoNotImplementThis {}

public protocol PrimitiveDataType: DoNotImplementThis {
    associatedtype Options = Void
}

internal typealias Primitive = DoNotImplementThis

public protocol CustomDataType {
    associatedtype UnderlyingPrimitive: PrimitiveDataType
    
    static var options: UnderlyingPrimitive.Options { get }
    static func fromPrimitive(_ value: UnderlyingPrimitive) -> Self
    var primitive: UnderlyingPrimitive { get }
}

// This is an example of a primitive data type
extension String: PrimitiveDataType {
    public enum DatabaseStringType {
        case char(length: Int), varchar(limit: Int), text
    }
    
    public typealias Options = DatabaseStringType
}

public protocol Table {}

public struct Column<T: Table, D: CustomDataType> {
    public let name: String
    
    public init(_ name: String) {
        self.name = name
    }
}

public struct Row<T: Table> {
    let columns: [String:Primitive]
    
    // this would normally be an internal initializer,
    // but for this demo I need to make it public.
    public init(_ columns: [String:DoNotImplementThis]) {
        self.columns = columns
    }
    
    public func get<CDT: CustomDataType>(_ column: Column<T, CDT>) -> CDT {
        let c = columns[column.name] as! CDT.UnderlyingPrimitive
        return CDT.fromPrimitive(c)
    }
}

And then this goes into the real playground file:

// Blabla.playground/Contents.swift
struct Name {
    let value: String
    
    init?(_ value: String) {
        guard case 0...50 = value.characters.count else {
            return nil
        }
        
        self.value = value
    }
}

extension Name: CustomDataType {
    static let options = String.Options.varchar(limit: 50)
    
    static func fromPrimitive(_ value: String) -> Name {
        return Name(value)!
    }
    
    var primitive: String {
        return value
    }
}

class Users: Table {
    static let name = Column<Users, Name>("name")
    
    // this is here just for the demo...
    static let first = Row<Users>(["name":"Evert"])
}

let user = Users.first
let name = Users.name
print(user.get(name))

This way it's impossible to ask for the wrong column or to ask for the right column from the wrong table, because if you make such a mistake, the app just won't compile. I agree it takes some more code to write, which I'm usually very much against, but in this case I just love how difficult it would be to write a bug here.

Evertt avatar Dec 25 '16 06:12 Evertt

+1 this

akshitzaveri avatar Oct 07 '18 16:10 akshitzaveri