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

Feature/separate value and metadata #90

Open
wants to merge 16 commits into
base: development
Choose a base branch
from

Conversation

JimRoepcke
Copy link

Dan, thanks for making YapDatabaseExtensions!

This PR contains some sutble but signficant changes I consider important for improving the performance and flexibility of this framework.

Metadata changes

This PR improves the handling metadata by YapDatabaseExtensions. I see there have been some previous attempts at reforming the metadata handling but none had been merged yet. I hope this solution is found to be acceptable.

The var metadata property and associatedtype MetadataType as been removed from the Persistable protocol.

YapDatabase does not place a constraint on the types of metadata that can be associated with values, and this is a useful feature. Having a single MetadataType per Persistable type reduced this flexibility.

An important performance design aspect of metadata in YapDatabase is the ability able to read and write it independently of the value. Unfortunately, with the current design, when you have a Persistable with a MetadataType, all reads will do an extra read for the metadata and then set the metadata on the value. This means twice the number of reads (even when the metadata isn't needed after the read) and creating a copy of the value when value types are used, as value types are immutable. If one used objects for their Yap values they'd have to make them mutable which isn't compatible with Yap's sharing policy. In practice, values and metadata do tend to be written together, but for reads the opposite is true.

It is still possible to read/write a value and a metadata at the same time - the methods in the "With[Object|Value]Metadata" files have been renamed to have "with metadata" in the name. For example, write is now writeWithMetadata. These "with metadata" methods take and return a YapItem<Value, Metadata>, which is a simple new struct akin to a tuple, containing the value, metadata and their associated type information since it's generic. This is preferable to a tuple because anonymous tuple types cannot be extended, while structs obviously can. By using this type-rich YapItem, the desirable type-safety features in YapDatabaseExtensions are not impacted.

Additional changes

Additionally, the methods that query for multiple YapDB.Index (such as readAtIndexes) no longer flatMap the return value, so it is now easy to determine which YapDB.Index were not found in the database. If one doesn't need this information they can easily flatMap the return value.

How to test

All tests have been updated and pass.

I merged danthorpe:development into my branch just before submitting this PR. It merged with no conflicts.

@danthorpe
Copy link
Owner

Hi @JimRoepcke - I'm sort of right in the middle of updating to Swift 3.0 at the moment. Will look through your proposal tomorrow. Thanks so much for your contribution!

@JimRoepcke
Copy link
Author

Thanks Dan - FYI I'm happy to reformulate my diff for Swift 3 once it's merged. Since the changes are so repetitive I can probably do most of them with a few regexes :)

@JimRoepcke
Copy link
Author

One thing I forgot to mention in the description is that with the removal of the metadata property, it's now easier to adapt any value type to becoming Persistable, because it's trivial to implement collection and identifier in an extension, but not possible to implement metadata in an extension.

This means we can keep our domain types free of Yap-isms, and add them at the layer of our architecture where we concern ourselves with persistence by introducing YapDatabase, ValueCoding and YapDatabaseExtensions.

@JimRoepcke JimRoepcke mentioned this pull request Nov 30, 2016
@danthorpe danthorpe added this to the 3.0 milestone Dec 7, 2016
@danthorpe danthorpe self-assigned this Dec 7, 2016
@JimRoepcke JimRoepcke force-pushed the feature/separate-value-and-metadata branch from 3162cf3 to 5014045 Compare January 2, 2017 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants