Skip to content
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

add re-usable Row<Model> class #12

Merged
merged 6 commits into from
Mar 21, 2019
Merged

add re-usable Row<Model> class #12

merged 6 commits into from
Mar 21, 2019

Conversation

tanner0101
Copy link
Member

@tanner0101 tanner0101 commented Mar 19, 2019

As discussed in #11, here is a proof of concept of a generic Row type. This allows for models to just declare their unique schema, without needing to re-implement the bare-bones of a model.

Before

Before the change, properties were declared in a nested Model.Properties struct.

final class Galaxy: Model {
    struct Properties: ModelProperties {
        let id = Field<Int>("id")
        let name = Field<String>("name")
        let planets = Children<Planet>(id: .init("galaxyID"))
    }
    static let properties = Properties()
    var storage: Storage
    init(storage: Storage) {
        self.storage = storage
    }
}

let galaxy = Galaxy()
galaxy.set(\.name, to: "Milky Way")
galaxy.save()

let galaxies = db.query(Galaxy.self).filter(\.name == "Milky Way").all().wait()
print(galaxies) // [Galaxy]

After

After the change, the model is the properties struct. A method new() is available to create a Row<Model>.

struct Galaxy: Model {
    static let `default` = Galaxy()
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let planets = Children<Planet>(id: .init("galaxyID"))
}

let galaxy = Galaxy.new() // Row<Galaxy>
galaxy.set(\.name, to: "Milky Way")
galaxy.save(on: db)

let galaxies = db.query(Galaxy.self).filter(\.name == "Milky Way").all().wait()
print(galaxies) // [Row<Galaxy>]

Note on dynamic KeyPath lookup

With Pitch: Key-Path Member Lookup possibly coming to Swift in the future, this change would allow us to declare Row as @keyPathMemberLookup and get back normal dot-syntax:

// with theoretical @keyPathMemberLookup 
let galaxy = Galaxy.new() // Row<Galaxy>
galaxy.name = "Milky Way"
galaxy.save(on: db)

Naming

I'm up for any ideas on how to spell Row and Model.default. Some other ideas I have:

Row:

  • Ref / Reference
  • Entity
  • Object
  • Instance

Model.default

  • shared
  • ref

@tanner0101 tanner0101 added the enhancement New feature or request label Mar 19, 2019
@tanner0101 tanner0101 added this to In Progress in Vapor 4 via automation Mar 19, 2019
@twof
Copy link
Member

twof commented Mar 19, 2019

let galaxies = db.query(Galaxy.self).filter(\.name == "Milky Way").all()
print(galaxies) // [Instance<Galaxy>]

Should that be Future<[Instance<Galaxy>]>?

@tanner0101
Copy link
Member Author

@twof oops, yeah I forgot .wait(). Added.

@tanner0101 tanner0101 changed the title add re-usable Instance<Model> class add re-usable Row<Model> class Mar 20, 2019
@twof
Copy link
Member

twof commented Mar 20, 2019

With this setup, are we at risk of people using and creating initializers? Is a raw Model object useful at all?

@tanner0101
Copy link
Member Author

@twof yes, it's possible people can call Planet() accidentally instead of Planet.new() and they will get a Model, not a Row<Model>. Model doesn't have any of the get / set methods though, so it should be immediately obvious that something is not right.

@tanner0101
Copy link
Member Author

Seems like feedback on this change is mostly positive. Given that the changes here would likely be a requirement to supporting @dyanmicMemberLookup in the future, I think we should definitely merge. 👍

@tanner0101 tanner0101 merged commit 44f1e9a into master Mar 21, 2019
Vapor 4 automation moved this from In Progress to Done Mar 21, 2019
@penny-coin
Copy link

Hey @tanner0101, you just merged a pull request, have a coin!

You now have 1154 coins.

@tanner0101 tanner0101 deleted the model-instance branch March 21, 2019 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants