starred/CoreStore
1
0
Fork 0
mirror of https://github.com/JohnEstropia/CoreStore.git synced 2026-08-29 06:37:27 +02:00
Code Issues 95 Packages Projects Releases 10 Wiki Activity

SynchronousDataTransaction commit() not marked @discardableResult #122

New Issue
Closed
opened 2025-12-29 15:24:56 +01:00 by adam · 4 comments
adam commented 2025-12-29 15:24:56 +01:00
Owner
Copy Link
Copy Source

Originally created by @avaroam on GitHub (Jan 25, 2017).

Just thought I'd mention that the commit() method of SynchronousDataTransaction is not marked with @discardableResult, meaning it must currently be used like this:

_ = transaction.commit()

I'm not sure if this is intentional, but it seems like something that would often not be used, and in the read me it is used without _ =

Also, same is true of commitAndWait()

Originally created by @avaroam on GitHub (Jan 25, 2017). Just thought I'd mention that the `commit()` method of `SynchronousDataTransaction` is not marked with `@discardableResult`, meaning it must currently be used like this: ``` _ = transaction.commit() ``` I'm not sure if this is intentional, but it seems like something that would often not be used, and in the read me it is used without `_ =` Also, same is true of `commitAndWait()`
adam closed this issue 2025-12-29 15:24:56 +01:00
adam commented 2025-12-29 15:24:57 +01:00
Author
Owner
Copy Link
Copy Source

@JohnEstropia commented on GitHub (Jan 26, 2017):

This was intentional because the result should generally be handled. Incidentally beginSynchronous() was made @discardableResult so that you can handle the result in one place. Maybe interchange the discardableResult attribute on those two methods?

@JohnEstropia commented on GitHub (Jan 26, 2017): This was intentional because the result should generally be handled. Incidentally `beginSynchronous()` was made `@discardableResult` so that you can handle the result in one place. Maybe interchange the discardableResult attribute on those two methods?
adam commented 2025-12-29 15:24:57 +01:00
Author
Owner
Copy Link
Copy Source

@avaroam commented on GitHub (Jan 26, 2017):

That makes sense! Having no discardable result on commit() makes it consistent with async transactions. Just thought I'd mention it because it's written without _ = in the README.

Out of curiosity, what would be the usual way to handle the SaveResult? What would be a use case for hasChanges, and is it really worth checking the CoreStoreError for all transactions? What would be a suitable course of action if an error did occur?

@avaroam commented on GitHub (Jan 26, 2017): That makes sense! Having no discardable result on `commit()` makes it consistent with async transactions. Just thought I'd mention it because it's written without `_ =` in the README. Out of curiosity, what would be the usual way to handle the `SaveResult`? What would be a use case for `hasChanges`, and is it really worth checking the `CoreStoreError` for all transactions? What would be a suitable course of action if an error did occur?
adam commented 2025-12-29 15:24:57 +01:00
Author
Owner
Copy Link
Copy Source

@JohnEstropia commented on GitHub (Jan 27, 2017):

Since transactions produce .success when there are no changes, the hasChanges is used when you need distinguish that case. This is useful in cases like data imports.

Of course it is up to you and your app if you need to handle these results or not. The syntax just makes it clear that you explicitly chose to do it.

@JohnEstropia commented on GitHub (Jan 27, 2017): Since transactions produce `.success` when there are no changes, the `hasChanges` is used when you need distinguish that case. This is useful in cases like data imports. Of course it is up to you and your app if you need to handle these results or not. The syntax just makes it clear that you explicitly chose to do it.
adam commented 2025-12-29 15:24:57 +01:00
Author
Owner
Copy Link
Copy Source

@avaroam commented on GitHub (Jan 27, 2017):

Nice. I've just been ignoring the result in most cases (if not all), but I can definitely see where it could be useful. Thanks for the insight!

@avaroam commented on GitHub (Jan 27, 2017): Nice. I've just been ignoring the result in most cases (if not all), but I can definitely see where it could be useful. Thanks for the insight!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
abandoned
corestore bug
discussion
duplicate
enhancement
fixed
fixed
help wanted
in progress
investigating
ios/compiler bug
irreproducible
looking for workarounds
not a bug
pending
pending docs update
pull-request
Mirrored from GitHub Pull Request
 question
regression
user bug
wontfix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/CoreStore#122