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

Use existsInDatabase property during save to allow overriding #110

Open
wants to merge 1 commit into
base: non-unique-instances
Choose a base branch
from

Conversation

jsclayton
Copy link

Small change to use the existsInDatabase property instead of checking the same thing.

The reason for the change is that I'd like to be able to have models inserted as new instead of updated when they have changes like so:

override var existsInDatabase: Bool {
    return super.existsInDatabase & !self.hasUnsavedChanges
}

override func save() -> Bool {
    if self.hasUnsavedChanges {
        self.loadId = Load.primaryKeyValueForNewInstance().longLongValue
    }
    return super.save()
}

@marcoarment
Copy link
Owner

To be clear, in your use-case, is loadId the primary key, so it's avoiding duplicate-insert errors by changing the primary key on every save?

Seems like a really bad idea that could cause weird bugs, since the assumption is that an instance's primary key value never changes after creation. Wouldn't a better pattern be to make a new instance and copy any relevant values from the old one?

@jsclayton
Copy link
Author

Yes loadId is the primary key. The goal is immutable objects to maintain a history. I'm loading the model, presenting an edit form, and if the user changed something saving a new revision instead of overwriting the prior version.

Copying values to a new object is the other option I'd considered, but with these non-unique instances changing the primary key on save and inserting seemed like it would be the least error prone approach.

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.

None yet

2 participants