Skip to content
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

refactor: Make bloc implementations private #1342

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

provokateurin
Copy link
Member

Some places accessed parts of Blocs that should have been in the interface. Now it is no longer possible to do that.

Copy link
Member

@Leptopoda Leptopoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the interfaces should be sealed.

  1. I don't like interface classes with constructors/factories
  2. We prevent our users to extend these interfaces.

Long term the concrete implementations should not contain private fields as the classes themselves are already private. You already did so for the TimerBloc. But please not in this PR as it's already large enoguh :)

packages/neon/neon_dashboard/lib/src/blocs/dashboard.dart Outdated Show resolved Hide resolved
packages/neon/neon_files/lib/src/blocs/browser.dart Outdated Show resolved Hide resolved
packages/neon/neon_files/lib/src/blocs/files.dart Outdated Show resolved Hide resolved
packages/neon/neon_news/lib/src/blocs/article.dart Outdated Show resolved Hide resolved
packages/neon_framework/lib/src/blocs/login_flow.dart Outdated Show resolved Hide resolved
packages/neon_framework/lib/src/blocs/first_launch.dart Outdated Show resolved Hide resolved
packages/neon_framework/lib/src/blocs/capabilities.dart Outdated Show resolved Hide resolved
@provokateurin
Copy link
Member Author

I don't like interface classes with constructors/factories

How should I fix it?

Long term the concrete implementations should not contain private fields as the classes themselves are already private

Agreed, I didn't do it to keep the diff more readable.

@Leptopoda
Copy link
Member

How should I fix it?

a sealed class can no longer have the interface modifier :)
So just make them sealed as suggested in the review.

@provokateurin
Copy link
Member Author

But that doesn't make the factories go away, no? I thought you saw that as a problem too.

@Leptopoda
Copy link
Member

I'm fine with them :)

@provokateurin
Copy link
Member Author

Making the classes sealed prevents us from creating mocks (at least with mocktail) :/

I think we should still go for it as we shouldn't need to have mocks for blocs once we untangle them.

@provokateurin provokateurin force-pushed the refactor/private-bloc-implementations branch from 3911cbc to e3da727 Compare December 23, 2023 08:45
@Leptopoda
Copy link
Member

Making the classes sealed prevents us from creating mocks (at least with mocktail) :/

I haven't tried it yet and don't know if it would help but meta also has a sealed annotation. This would only produce lint warnings instead of compile time errors when one tries to extend it. Maybe this is a better idea 🤔

@provokateurin provokateurin force-pushed the refactor/private-bloc-implementations branch from 0cd0bc2 to c1e02f4 Compare December 27, 2023 09:28
@Leptopoda
Copy link
Member

I'll review this later today

@provokateurin provokateurin force-pushed the refactor/private-bloc-implementations branch from c1e02f4 to 8fc3cd0 Compare December 28, 2023 15:07
@provokateurin provokateurin merged commit d2a8b18 into main Dec 28, 2023
8 checks passed
@provokateurin provokateurin deleted the refactor/private-bloc-implementations branch December 28, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants