Add fromJson and toJson fields to JsonKey class
#146
Conversation
kevmoo
force-pushed
the
i137_convert
branch
3 times, most recently
from
b58dd15 to
d4edbe5
Compare
kevmoo
changed the title
fromJsonFunction and toJsonFunction fields to JsonKey class
| this.nullable, | ||
| this.includeIfNull, | ||
| this.ignore, | ||
| this.fromJsonFunction, |
There was a problem hiding this comment.
can we pick shorter names? fromJson and toJson would not be ambiguous in this context and I think matches with with toJson methods on an object.
| nullable: obj.getField('nullable').toBoolValue(), | ||
| includeIfNull: obj.getField('includeIfNull').toBoolValue(), | ||
| ignore: obj.getField('ignore').toBoolValue()); | ||
| if (obj == null) { |
There was a problem hiding this comment.
FWIW I liked the ternary conditional better, especially given the shortened form. I like if for when the side effect is meaningful and ternary for when the assignment is meaningful.
There was a problem hiding this comment.
made it even better...
| toJsonName); | ||
| } | ||
|
|
||
| class ConvertDataImpl { |
There was a problem hiding this comment.
what is Impl adding for this name?
There was a problem hiding this comment.
Impl to scream that this class shouldn't be in the public API.
But...whatever...
| ConvertDataImpl(this.name, this.paramType); | ||
| } | ||
|
|
||
| class JsonKeyImpl extends JsonKey { |
There was a problem hiding this comment.
Can we find a more meaningful distinction from the parent class for this name? JsonKeyWithConversion or something?
kevmoo
changed the title
fromJsonFunction and toJsonFunction fields to JsonKey classfromJson and toJson fields to JsonKey class
No description provided.