Type helper cleanup, library reorganizations #21
Conversation
kevmoo
force-pushed
the
type_helper_more
branch
3 times, most recently
from
324ec97 to
8818d8f
Compare
kevmoo
changed the title
|
@natebosch it's big, but it's really nice CC @jakemac53 |
You can reuse the same name – `e` in our case – at each level It makes reading the generated code a bit harder – so use tools
|
@natebosch – rebased on latest |
| @@ -1,7 +1,27 @@ | |||
| ## 0.2.0 | |||
|
|
|||
| * **BREAKING** Types are now segmented into their own libraries. | |||
|
|
|||
There was a problem hiding this comment.
[nit] why the blank lines? (just realized they've been in there from the start...)
There was a problem hiding this comment.
A bit easier to read the source and the parsed markdown.
| import 'utils.dart'; | ||
|
|
||
| final _substitute = "e"; |
There was a problem hiding this comment.
[nit] maybe a comment or a more descriptive variable name?
| static const _defaultHelpers = const [ | ||
| const JsonHelper(), | ||
| const DateTimeHelper() | ||
| const DateTimeHelper(), | ||
| const CoreHelper() |
There was a problem hiding this comment.
it feel odd that there is both a _coreHelpers and a CoreHelper()...
There was a problem hiding this comment.
Done. Also caught a bug! – CoreHelper should be in _coreHelpers!
| valueType.isObject || | ||
| _stringBoolNumChecker.isAssignableFromType(valueType); | ||
| String _serialize(DartType targetType, String expression) => _allHelpers | ||
| .map((th) => th.serialize(targetType, expression, _serialize)) |
There was a problem hiding this comment.
TypeHelper – would you prefer e?
There was a problem hiding this comment.
Maybe just h for 'helper' coming from _allHelpers -> or change it to _allTypeHelpers and keep th
lib/src/type_helper.dart
Outdated
| /// returns the generic arguments to the [checker] [Type] if there are any. | ||
| /// | ||
| /// If the [checker] [Type] doesn't have generic arguments, `null` is returned. | ||
| List<DartType> getTypeArguments(DartType type, TypeChecker checker) { |
There was a problem hiding this comment.
[nit] could be typeArguments or typeArgumentsOf... get doesn't add value most of the time.
lib/src/type_helper.dart
Outdated
| // the time. | ||
| @override | ||
| bool canSerialize(DartType type) => canDeserialize(type); | ||
| /// A [TypeChecker] for [String]. |
There was a problem hiding this comment.
[nit] Is the comment adding new information?
BTW a pattern I've seen Matan use with TypeChecker is to name the variable like $String which makes the usage look nicer than repeating TypeChecker
There was a problem hiding this comment.
Uh...I think this is deleted code...
There was a problem hiding this comment.
Nevermind. Found it. Uh...it makes it clear what it's for.
Better a trivial doc comment than no doc comment. This is a public API.
Not sure where you're going with the $String thing...
kevmoo
force-pushed
the
type_helper_more
branch
2 times, most recently
from
69d86d8 to
2eb9284
Compare
Allows `Iterable`, `List`, and `Map` to be implemented as helpers Allow rename and reorganize public libraries.
Reorganize libraries so there is one clear import for each use case.
Support nesting in
TypeHelper.Cleanup the API for
TypeHelperOther misc