-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Follow best practice of ProxyProvider updating #298
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
Conversation
The addresses feedback from flutter#245. Instead of constructing a new `CartModel`, we merely update the `catalog` field. We no longer need a fancy constructor, and `CartModel._catalog` cannot be final any more.
CartModel(catalog, previousCart), | ||
create: (context) => CartModel(), | ||
update: (context, catalog, cart) { | ||
cart.catalog = catalog; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit but I'd use the ..
operator instead of making a block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To offer a counterpoint, in a sample that's aimed at beginners (like this one), we sometimes stretch out the syntax to maximize readability for new folks.
Entirely your call which way to go, though, Filip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I had the cascade operator there at first but went with the more verbose syntax for the reason @RedBrogdon mentions. We're not teaching Dart niceties here, so I go with as "vanilla" syntax as possible, so as to decrease cognitive load on the beginner user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@filiph @RedBrogdon Thanks a ton, As a beginner user it's very hard to understand these kind of syntax ..
vanilla syntax rocks:)
CartModel(catalog, previousCart), | ||
create: (context) => CartModel(), | ||
update: (context, catalog, cart) { | ||
cart.catalog = catalog; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To offer a counterpoint, in a sample that's aimed at beginners (like this one), we sometimes stretch out the syntax to maximize readability for new folks.
Entirely your call which way to go, though, Filip.
The addresses feedback from #245. Instead of constructing a new
CartModel
, we merely update thecatalog
field.We no longer need a fancy constructor, and
CartModel._catalog
cannot be final any more.This change also upgrades to
pkg:provider
4.0.2, just to stay on top of things (it's was a no-op in the case of this sample).