Add Firestore transaction example #442
Conversation
packages/cloud_firestore/README.md
Outdated
| final DocumentReference postRef = Firestore.instance.document('posts/123'); | ||
| final TransactionHandler transactionHandler = (Transaction tx) async { | ||
| DocumentSnapshot postSnapshot = await tx.get(postRef); | ||
| if (postSnapshot.exist) { |
There was a problem hiding this comment.
I think you mean exists
packages/cloud_firestore/README.md
Outdated
| @@ -60,6 +60,19 @@ class BookList extends StatelessWidget { | |||
| } | |||
| ``` | |||
|
|
|||
| Running a transaction | |||
There was a problem hiding this comment.
I would put a colon here for consistency with the other examples in this section
packages/cloud_firestore/README.md
Outdated
|
|
||
| ```dart | ||
| final DocumentReference postRef = Firestore.instance.document('posts/123'); | ||
| final TransactionHandler transactionHandler = (Transaction tx) async { |
There was a problem hiding this comment.
Assigning functions to variables this way is against the Dart style guide (not sure about Flutter)
Any reason why this is not passed as a closure or at least a nested function?
See also this linter rule http://dart-lang.github.io/linter/lints/prefer_function_declarations_over_variables.html
There was a problem hiding this comment.
With sound dart or dart 2 on the horizon, the goal is to express the intent of the type, so with function expressions it is good to express the consumed type on the left side. So with that said, it seems reasonable to to express clearly the expressed type, so the reader can clearly read the story with in moments of landing.
https://www.dartlang.org/guides/language/effective-dart/design#avoid-annotating-types-on-function-expressions
There was a problem hiding this comment.
In this particular case I think it would be concise and readable to define a closure at the point of invoking runTransaction(), like we do for setState(). @kroikie, do you agree? i.e.
Firestore.instance.runTransaction((Transaction tx) async { ... });
Update README per comments
|
What is your plan with this PR? (No particular rush, I'm just cleaning out the review queue.) |
fixes: flutter/flutter#14655
Adding a transaction example to README.