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

default implementation for serialize #30

Closed
Evertt opened this issue May 11, 2016 · 11 comments
Closed

default implementation for serialize #30

Evertt opened this issue May 11, 2016 · 11 comments
Labels
enhancement New feature or request

Comments

@Evertt
Copy link
Contributor

Evertt commented May 11, 2016

The only reason I'm suggesting this is because I believe we can implement this functionality ourselves, without requiring the creator of any Entity to implement it. I already wrote some code to do this.

func serialize(entity: Entity, fields: [String]) -> [String:Value?] {
    let entityMirror = Mirror(reflecting: entity)
    var serialized = [String:Value?]()

    for child in entityMirror.children {
        // Match the property if it's in the provided fields array
        if let property = child.label where fields.contains(property) {
            switch unwrap(child.value) {

            // If it's another entity, then it must be a belongsTo relationship
            case let entity as Entity:
                if let id = entity.id {
                    serialized[property] = id
                }

            // Otherwise it must just be some value
            case let value as Value?:
                serialized[property] = value

            // If we get here then something is off
            default: fatalError("Fields must be either a `Value` or an `Entity`")
            }
        }
    }

    return serialized
}

// Don't mind this function.
// It just unwraps a value that's potentially secretly an optional to a real optional.
func unwrap(any: Any) -> Any? {
    let mirror = Mirror(reflecting: any)

    if mirror.displayStyle != .Optional {
        return any
    }

    if mirror.children.count == 0 {
        return nil
    }

    let (_, some) = mirror.children.first!

    return some
}

Now we just need some method of determining which columns need to be extracted. But I think someone was already working on a SchemaBuilder and such right? #18

What do you guys think? My preference is to require as little as possible in the Entity protocol. If I could also implement a func unserialize(serialized: [String:Value?]) -> Entity then I would, but unfortunately that's not possible without turning every Entity into an NSObject.

@tanner0101
Copy link
Member

I'm pretty sure Mirror doesn't work on Linux because it relies on the Objective-C runtime. But we should double check.

@Evertt
Copy link
Contributor Author

Evertt commented May 11, 2016

Hmm, that would be a pity. I'll google it.

@Evertt
Copy link
Contributor Author

Evertt commented May 11, 2016

I haven't found any definitive answer yet in Apple documentation, but so far I've seen some stuff that seems to suggest that you don't need Obj-C runtime for this.

@tannernelson would you by any chance have a linux vm on your computer which you use to test your code? If so, could you just try to compile that function on that vm? I guess that would be the surest way to answer that question.

@tanner0101
Copy link
Member

Looks like at least getting the names of the properties works actually on my Linux box.

class Test {
        var prop = ""
        init() { }
}

let test = Test()

let m = Mirror(reflecting: test)
print(m)

for c in m.children {
        print(c)
}

prints

tanner@Qutheory:~/reflection$ ./main 
Mirror for Test
(Optional("prop"), "")

So we could potentially do this. My concern would be how to differentiate properties that should be stored in the database from ones that shouldn't. Such as computed properties or others.

@Evertt
Copy link
Contributor Author

Evertt commented May 12, 2016

That's why we need another class that is aware of the database schema. And I thought somebody was already working on that, right?

And then the developers just need to follow the convention that they mirror table field names exactly in their entity property names. Which is a good practice anyway, I think.

@tanner0101
Copy link
Member

So entity would be a mirror of the DB schema?

@Evertt
Copy link
Contributor Author

Evertt commented May 12, 2016

Yes, at least in the sense that it would have at least all the properties that match the database table columns. But of course the Entity is allowed to have more properties, like computed ones.

@Evertt Evertt mentioned this issue May 13, 2016
@tanner0101 tanner0101 added enhancement New feature or request reviewing labels May 18, 2016
@tanner0101 tanner0101 changed the title Take the serialize() -> [String: Value?] requirement out of the Entity protocol default implementation for serialize May 18, 2016
@tanner0101
Copy link
Member

Things have changed a bit since this issue was made, so let's revisit it once I merge the refactor of Fluent that works with both SQLite and Mongo.

@Joannis
Copy link
Member

Joannis commented May 18, 2016

https://github.com/Zewo/reflection It does work under Linux. Both reading as well as writing.

@loganwright
Copy link
Member

I would personally opt to leave reflection out of it. For me, reflection is a debugging tool that will likely be inconsistent to rely on.

I recognize it's a little bit more work up front, but safety is one of the things we want to encourage and utilize in Swift.

With good mappers, this is usually very minimal and the user has to decode from data to object either way.

@tanner0101
Copy link
Member

We tried reflection but it was very inconsistent like logan said. Let's wait for Swift 4 when we will have an improved Mirror class.

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
None yet
Development

No branches or pull requests

4 participants