Leandro Forte - Flutter Coding Test #75

leandromichelforte · 2024-02-08T19:58:26Z

Hey guys,

Hope this message finds you well
The documentation is the app-documentation.md file in the project root folder

Thank you for the opportunity
See you later

…ter_create [PROJECT] Flutter Create

…er (pages and cubits)

…ix tests

[FEATURE][Flutter] QR Code Scan

[FEATURE] NodeJS Backend

…source and adjusting its tests

…_node [FEATURE] Connect Flutter to Node

[FEATURE] Add NodeJS backend and connect Flutter into it

[BUGFIX] Expires At

[PROJECT] Documentation

diegog-sf

Great work!

I think you have a solid understanding of domain layers, their responsibilities, and how they interact with each other. It is great to see that you tested all layers as well.

On the presentation side, I saw attention to detail and code organization with the cubit responsibilities.

diegog-sf · 2024-02-09T15:53:41Z

pubspec.yaml

+  cupertino_icons: ^1.0.2
+  bloc: ^8.1.2
+  flutter_bloc: ^8.1.3
+  bloc_test: ^9.1.5


bloc_test should be a dev dependency

You're right. I'll pay more attention to that. Thanks.

diegog-sf · 2024-02-09T15:53:57Z

pubspec.yaml

+  bloc_test: ^9.1.5
+  dartz: ^0.10.1
+  equatable: ^2.0.5
+  mocktail: ^1.0.3


mocktail should be a dev dependency

You're right. I'll pay more attention to that. Thanks.

diegog-sf · 2024-02-09T15:55:25Z

lib/core/extensions/build_context_extension.dart

+
+extension BuildContextExtension on BuildContext {
+  double get screenWidth => MediaQuery.of(this).size.width;
+  TextTheme get textTheme => Theme.of(this).textTheme;


kudos: nice!

diegog-sf · 2024-02-09T15:55:57Z

lib/core/injections/dio_inject.dart

+
+sealed class DioInject {
+  static Future<void> inject() async {
+    getIt.registerFactory<Dio>(


question: why a factory and not a singleton?

You're right, I should have used singleton for that knowing the singleton will create the instance only when called.
I'll pay more attention to that. Thanks.

diegog-sf · 2024-02-09T16:04:42Z

lib/features/qr_code_scan/presenter/pages/qr_code/qr_code_page.dart

+
+  @override
+  void dispose() {
+    _timerCubit.disposeTimer();


thought: great catch disposing the timer if disposing the screen, butyou can also dispose the timer on the cubit's dispose method.

Yeah, of course. Thanks.

diegog-sf · 2024-02-09T16:04:55Z

lib/features/qr_code_scan/presenter/pages/qr_code/widgets/shimmer_widget.dart

+  final double dimension;
+
+  @override
+  Widget build(BuildContext context) {


kudos: nice laoding animation

diegog-sf · 2024-02-09T16:05:14Z

lib/features/qr_code_scan/presenter/pages/qr_code/widgets/qr_code_loaded_body_widget.dart

+          BlocBuilder<TimerCubit, TimerState>(
+            bloc: _timerCubit,
+            builder: (context, state) {
+              return switch (state) {


kudos: dart 3 ussage

diegog-sf · 2024-02-09T16:12:34Z

test/features/qr_code_scan/domain/usecases/get_qr_code/get_qr_code_usecase_impl_test.dart

+
+    test(
+        'should get QR code from the repository and check if it was called once',
+        () async {


kudos: for testing all layers

laurent-sf · 2024-02-09T16:37:39Z

lib/features/qr_code_scan/domain/repositories/get_qr_code_repository.dart

question: Based on the path (lib/features/qr_code_scan/domain/repositories), do you consider that repositories belong to the domain layer?

Based on the Clean Architecture concept, I know the repository belongs to the data layer, but I like to create only the abstract class in the domain folder and the implementation class in the data folder just to represent the connection between the domain and data layers.

Thanks.

laurent-sf · 2024-02-09T16:41:37Z

lib/features/qr_code_scan/domain/repositories/get_qr_code_repository.dart

+import 'package:superformula_leandro/core/errors/failures/failure.dart';
+import 'package:superformula_leandro/features/qr_code_scan/domain/entities/qr_code_entity.dart';
+
+abstract interface class GetQrCodeRepository {


question: Could you explain why you named the repository GetQrCodeRepository and not just QrCodeRepository?

You're right. I forgot to rename it. I'll pay more attention to that. Thanks.

laurent-sf · 2024-02-09T16:55:39Z

lib/features/qr_code_scan/presenter/pages/scan/scan_page.dart

+import 'package:superformula_leandro/features/qr_code_scan/presenter/cubits/scan/scan_cubit.dart';
+import 'package:superformula_leandro/features/qr_code_scan/presenter/pages/scan/widgets/scanned_data_body_widget.dart';
+
+class ScanPage extends StatefulWidget {


question: Could you explain why you preferred using a StatefulWidget instead of an StatelessWidget?

Just to initialize the ScanCubt from the context.read and after call the startScan method only in the beginning of the page lifecycle.

Theres another way to do that, using StatelessWidget and calling the startScan method when the state is the ScanInitialState.

Thanks.

What about doing it when instantiating the cubit? Do you see any inconvenient with this approach?

BlocProvider<ScanCubit>( create: (context) => ScanCubit()..startScan(), child: const ScanPage(), ),

No inconvenient at all. That's a really good way to do that as well. Thanks.

laurent-sf · 2024-02-09T16:57:14Z

lib/features/qr_code_scan/presenter/pages/scan/scan_page.dart

+        padding: const EdgeInsets.all(16),
+        child: Center(
+          child: BlocBuilder<ScanCubit, ScanState>(
+            bloc: _scanCubit,


question: Do you think it's necessary to pass bloc as parameter?

I like to use this parameter to make the understanding of the code easier. For example, if a page has a lot of cubits, this parameter I'll helps who is reading to understand what that widget is responsible for.

Thanks.

laurent-sf · 2024-02-09T16:58:18Z

lib/features/qr_code_scan/presenter/pages/scan/scan_page.dart

+        ),
+      ),
+      bottomNavigationBar: BlocBuilder<ScanCubit, ScanState>(
+        bloc: _scanCubit,


suggestion: Maybe you could have used buildWhen to avoid unnecessary rebuilds.

I don't think it's necessary because in this page, I need to rebuild this bottomsheet every time cubit emits something.

I have the emitted states: ScanningState, ScanErrorState and ScannedDataState and I need to only show when It's ScannedDataState.

For example, If I've had the buildWhen: (_, current) => current is ScannedDataState, when emits another state the bottomsheet will not disappear and I don't want it.

Don't you think so?

Anyway, thanks for noticing that.

Maybe we could use the following condition, what do you think?

buildWhen: (previous, current) { return (previous is ScannedDataState) ^ (current is ScannedDataState); },

Yess. This should work, you're right. Thanks.

leandromichelforte · 2024-02-12T16:36:50Z

Hi guys. I replied to all your comments.

I really appreciate all your feedbacks and I'm looking forward for the next steps.

Feel free to reach me out to anything. Thanks one more time.

Leandro Forte and others added 26 commits February 6, 2024 15:25

create flutter project

2c3fce1

remove unnecessary comments

f80178a

Merge pull request #1 from leandromichelforte/merge-main/project/flut…

480508e

…ter_create [PROJECT] Flutter Create

add core folder; home, qr_code and scan: added tests on presenter lay…

61742cc

…er (pages and cubits)

creating get qr code usecase, repository, datasource and its tests

084b9d2

finishing qr code page, cubit and its tests

64dcd2e

deal with permission on scan page, refactor widgets and strings and f…

3afb43c

…ix tests

Merge pull request #2 from leandromichelforte/feature/qr_code_scan

144f12c

[FEATURE][Flutter] QR Code Scan

Merge pull request #3 from leandromichelforte/develop

b30cb90

[FEATURE][Flutter] QR Code Scan

add node js backend to return the seed

1fd723c

Merge pull request #4 from leandromichelforte/feature/backend

3bfaa4a

[FEATURE] NodeJS Backend

refactor inject, add dio, retrieve from node backend and fix tests

6b8e84d

rename from expireAt to expiresAt

dcf2bb6

change key value from returned value from expireAt to expiresAt

b01cff4

add generic and api exception, throws news exceptions in qr code data…

1074306

…source and adjusting its tests

Merge pull request #5 from leandromichelforte/feature/connect_flutter…

a4a3429

…_node [FEATURE] Connect Flutter to Node

Merge pull request #6 from leandromichelforte/develop

999e1ab

[FEATURE] Add NodeJS backend and connect Flutter into it

remove time zone offset from expires_at and add 1 minute from now

e1a5dd0

Merge pull request #7 from leandromichelforte/bugfix/expires_at

28bf721

[BUGFIX] Expires At

Merge pull request #8 from leandromichelforte/master

23e1235

[BUGFIX] Expires At

add app documentation

7efc17c

Merge pull request #9 from leandromichelforte/project/documentation

300b593

[PROJECT] Documentation

Merge pull request #10 from leandromichelforte/develop

687078d

[PROJECT] Documentation

adjust tab in app doc md file

1ec2ecc

Merge pull request #11 from leandromichelforte/project/documentation

56204d1

[PROJECT] Documentation

Merge pull request #12 from leandromichelforte/develop

4b5ce96

[PROJECT] Documentation

diegog-sf reviewed Feb 9, 2024

View reviewed changes

laurent-sf reviewed Feb 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leandro Forte - Flutter Coding Test #75

Leandro Forte - Flutter Coding Test #75

leandromichelforte commented Feb 8, 2024

diegog-sf left a comment

diegog-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

diegog-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

diegog-sf Feb 9, 2024

diegog-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

diegog-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

diegog-sf Feb 9, 2024

diegog-sf Feb 9, 2024

diegog-sf Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

laurent-sf Feb 9, 2024

leandromichelforte Feb 9, 2024

leandromichelforte commented Feb 12, 2024

Leandro Forte - Flutter Coding Test #75

Are you sure you want to change the base?

Leandro Forte - Flutter Coding Test #75

Conversation

leandromichelforte commented Feb 8, 2024

diegog-sf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandromichelforte commented Feb 12, 2024