SynchronousDataTransaction commit() not marked @discardableResult #122

Closed
opened 2025-12-29 15:24:56 +01:00 by adam · 4 comments
Owner

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
Author
Owner

@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?
Author
Owner

@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?
Author
Owner

@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.
Author
Owner

@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.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/CoreStore#122