Skip to content

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

Merged
merged 2 commits into from
Jan 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions provider_shopper/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ class MyApp extends StatelessWidget {
// of ChangeNotifierProvider. Moreover, CartModel depends
// on CatalogModel, so a ProxyProvider is needed.
ChangeNotifierProxyProvider<CatalogModel, CartModel>(
create: (context) => CartModel.empty(),
update: (context, catalog, previousCart) =>
CartModel(catalog, previousCart),
create: (context) => CartModel(),
update: (context, catalog, cart) {
cart.catalog = catalog;

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

Copy link
Contributor

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.

Copy link
Contributor Author

@filiph filiph Jan 31, 2020

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.

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:)

return cart;
},
),
],
child: MaterialApp(
Expand Down
34 changes: 17 additions & 17 deletions provider_shopper/lib/models/cart.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,25 @@ import 'package:flutter/foundation.dart';
import 'package:provider_shopper/models/catalog.dart';

class CartModel extends ChangeNotifier {
/// The current catalog. Used to construct items from numeric ids.
final CatalogModel _catalog;
/// The private field backing [catalog].
CatalogModel _catalog;

/// Internal, private state of the cart. Stores the ids of each item.
final List<int> _itemIds;

/// Construct a CartModel instance that is backed by a [CatalogModel] and
/// an optional previous state of the cart.
///
/// If [previous] is not `null`, its items are copied to the newly
/// constructed instance.
CartModel(this._catalog, CartModel previous)
: assert(_catalog != null),
_itemIds = previous?._itemIds ?? [];

/// An empty cart with no Catalog.
CartModel.empty()
: _catalog = null,
_itemIds = [];
final List<int> _itemIds = [];

/// The current catalog. Used to construct items from numeric ids.
CatalogModel get catalog => _catalog;

set catalog(CatalogModel newCatalog) {
assert(newCatalog != null);
assert(_itemIds.every((id) => newCatalog.getById(id) != null),
'The catalog $newCatalog does not have one of $_itemIds in it.');
_catalog = newCatalog;
// Notify listeners, in case the new catalog provides information
// different from the previous one. For example, availability of an item
// might have changed.
notifyListeners();
}

/// List of items in the cart.
List<Item> get items => _itemIds.map((id) => _catalog.getById(id)).toList();
Expand Down
10 changes: 9 additions & 1 deletion provider_shopper/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ packages:
url: "https://pub.dartlang.org"
source: hosted
version: "1.1.8"
nested:
dependency: transitive
description:
name: nested
url: "https://pub.dartlang.org"
source: hosted
version: "0.0.4"
path:
dependency: transitive
description:
Expand Down Expand Up @@ -115,7 +122,7 @@ packages:
name: provider
url: "https://pub.dartlang.org"
source: hosted
version: "3.2.0"
version: "4.0.2"
quiver:
dependency: transitive
description:
Expand Down Expand Up @@ -193,3 +200,4 @@ packages:
version: "3.5.0"
sdks:
dart: ">=2.5.0 <3.0.0"
flutter: ">=1.12.1"
2 changes: 1 addition & 1 deletion provider_shopper/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ dependencies:
sdk: flutter

# Import the provider package.
provider: ^3.1.0
provider: ^4.0.2

dev_dependencies:
flutter_test:
Expand Down