Add read write to example app #237
Conversation
| late String readOutput; | ||
| late String writeOutput; | ||
| late String subscribeOutput; | ||
| late TextEditingController textEditingController; |
There was a problem hiding this comment.
Curious, I didn't know about this new keyword late 👍
| return textEditingController.text | ||
| .split(',') | ||
| .map( | ||
| (value) => int.parse(value), |
There was a problem hiding this comment.
Is it clear in UI that this is a decimal number input?
| final specificCharacteristicValueStream = characteristicValueStream | ||
| .where((update) => update.characteristic == characteristic) | ||
| .map((update) => update.result.dematerialize()); | ||
| .map((update) => update.result.dematerialize()!); |
There was a problem hiding this comment.
Is this result of type Result<X?>? If it's just Result<T>, then .dematerialize() should return a non-optional type, so that force-unwrapping is not needed.
Please, check this.
lib/src/model/result.dart
Outdated
| /// Provides the value in case of success or throws [Exception] in case of failure. | ||
| Value dematerialize() => iif( | ||
| success: (value) => value!, | ||
| Value? dematerialize() => iif( |
There was a problem hiding this comment.
No way this is right. The type should be non-optional.
Please revert.
There was a problem hiding this comment.
I know this is crap but can you explain me how we should deal with Result<void, GenericFailure<Faliure>> ? Because this returns an object of Result(value: null, failure: null) and thus crash. Maybe we should use Optional.none for this
There was a problem hiding this comment.
This is sort of a language design issue. The fact that the type void has the only possible instance, which is null, is self-contradicting: you can have a non-optional void type, but it's going to contain null!
Despite being named void, this type in Dart (as well as in other languages, take C for example) has a structure of a "unit" type, which means a type that only has a single possible instance (hence "unit"). A type with no possible instances should be called "void" (like Never in Swift), it is not possible to have an instance of such type (it is still useful for functions that never return [like fatalError(·)] and you can tell that from their type, that's really cool).
But that's weird naming with historical reasons.
I hope you see the contradiction in Dart's "sound null-safety" here 🙂
The thing is, here we need a proper "unit" type with a single possible instance, which is not null.
We already have such a type, see lib/src/model/unit.dart, and it's used in PluginController.clearGattCache(·).
I would suggest to start using the Unit more in public types (in public interfaces), because with Dart null-safety it apparently becomes a necessity.
There was a problem hiding this comment.
damm I was thinking yesterday night if only we had unit 🤦
|
Thanks for the review comments I think I adressed all the points (also made the input on the ui bit better) |
werediver
left a comment
werediver
left a comment
There was a problem hiding this comment.
Now looks good 👍 Nice to see the example app improving.
@werediver I did use a basic implementation not architectural wonders. Maybe in the future we can move to riverpod or BLOC to have more proper state management. For now I think this helps users. I discovered already one issue in relation to demterialize in null-safety which I fixed.
Screen.Recording.2021-03-07.at.19.22.09.mov
Fixes #226