Skip to content

Commit

Permalink
chore: standardize tabs usages
Browse files Browse the repository at this point in the history
  • Loading branch information
@marwfair
marwfair committed Sep 16, 2024
1 parent b707d44 commit 10f8cd8
Show file tree
Hide file tree
Showing 9 changed files with 1,051 additions and 1,132 deletions.
141 changes: 71 additions & 70 deletions src/content/docs/architecture/index.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,88 +111,89 @@ Each layer abstracts the underlying layers' implementation details. Avoid indire

When using layered architecture, data should only flow from the bottom up, and a layer can only access the layer directly beneath it. For example, the `LoginPage` should never directly access the `ApiClient`, or the `ApiClient` should not be dependent on the `UserRepository`. With this approach, each layer has a specific responsibility and can be tested in isolation.

Good ✅

```dart
class LoginPage extends StatelessWidget {
...
LoginButton(
onPressed: => context.read<LoginBloc>().add(const LoginSubmitted());
)
...
}
class LoginBloc extends Bloc<LoginEvent, LoginState> {
...
Future<void> _onLoginSubmitted(
LoginSubmitted event,
Emitter<LoginState> emit,
) async {
try {
await _userRepository.logIn(state.email, state.password);
emit(const LoginSuccess());
} catch (error, stackTrace) {
addError(error, stackTrace);
emit(const LoginFailure());
<Tabs>
<TabItem label="Good ✅">
```dart
class LoginPage extends StatelessWidget {
...
LoginButton(
onPressed: => context.read<LoginBloc>().add(const LoginSubmitted());
)
...
}
class LoginBloc extends Bloc<LoginEvent, LoginState> {
...
Future<void> _onLoginSubmitted(
LoginSubmitted event,
Emitter<LoginState> emit,
) async {
try {
await _userRepository.logIn(state.email, state.password);
emit(const LoginSuccess());
} catch (error, stackTrace) {
addError(error, stackTrace);
emit(const LoginFailure());
}
}
}
}
class UserRepository {
const UserRepository(this.apiClient);
class UserRepository {
const UserRepository(this.apiClient);
final ApiClient apiClient;
final ApiClient apiClient;
final String loginUrl = '/login';
final String loginUrl = '/login';
Future<void> logIn(String email, String password) {
await apiClient.makeRequest(
url: loginUrl,
data: {
'email': email,
'password': password,
},
);
}
}
```

Bad ❗️

```dart
class LoginPage extends StatelessWidget {
...
LoginButton(
onPressed: => context.read<LoginBloc>().add(const LoginSubmitted());
)
...
}
class LoginBloc extends Bloc<LoginEvent, LoginState> {
...
final String loginUrl = '/login';
Future<void> _onLoginSubmitted(
LoginSubmitted event,
Emitter<LoginState> emit,
) async {
try {
Future<void> logIn(String email, String password) {
await apiClient.makeRequest(
url: loginUrl,
data: {
'email': state.email,
'password': state.password,
'email': email,
'password': password,
},
);
);
}
}
```
</TabItem>
<TabItem label="Bad ❗️">
```dart
class LoginPage extends StatelessWidget {
...
LoginButton(
onPressed: => context.read<LoginBloc>().add(const LoginSubmitted());
)
...
}
class LoginBloc extends Bloc<LoginEvent, LoginState> {
...
final String loginUrl = '/login';
emit(const LoginSuccess());
} catch (error, stackTrace) {
addError(error, stackTrace);
emit(const LoginFailure());
Future<void> _onLoginSubmitted(
LoginSubmitted event,
Emitter<LoginState> emit,
) async {
try {
await apiClient.makeRequest(
url: loginUrl,
data: {
'email': state.email,
'password': state.password,
},
);
emit(const LoginSuccess());
} catch (error, stackTrace) {
addError(error, stackTrace);
emit(const LoginFailure());
}
}
}
}
```
```
</TabItem>
</Tabs>

In this example, the API implementation details are now leaked and made known to the bloc. The API's login url and request information should only be known to the `UserRepository`. Also, the `ApiClient` instance will have to be provided directly to the bloc. If the `ApiClient` ever changes, every bloc that relies on the `ApiClient` will need to be updated and retested.
45 changes: 26 additions & 19 deletions src/content/docs/code_style/code_style.md → src/content/docs/code_style/code_style.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,48 @@ title: Code Style
description: Best practices for general code styling that goes beyond linter rules.
---

import { TabItem, Tabs } from "@astrojs/starlight/components";

In general, the best guides for code style are the [Effective Dart](https://dart.dev/effective-dart) guidelines and the linter rules set up in [very_good_analysis](https://pub.dev/packages/very_good_analysis). However, there are certain practices we've learned outside of these two places that will make code more maintainable.

## Record Types

Among other things, the release of Dart 3.0 introduced [record types](https://dart.dev/language/records), a way to store two different but related pieces of data without creating a separate data class. When using record types, be sure to choose expressive names for positional values.

Bad ❗️
<Tabs>
<TabItem label="Bad ❗️">
```dart
Future<(String, String)> getUserNameAndEmail() async => return _someApiFetchMethod();
```dart
Future<(String, String)> getUserNameAndEmail() async => return _someApiFetchMethod();
final userData = await getUserNameAndEmail();
final userData = await getUserNameAndEmail();
// a bunch of other code...
// a bunch of other code...
if (userData.$1.isValid) {
// do stuff
}
```

if (userData.$1.isValid) {
// do stuff
}
```
</TabItem>
</Tabs>

The above example will compile, but it is not immediately obvious what value `userData.$1` refers to here. The name of the function gives the reader the impression that the second value in the record is the email, but it is not clear. Particularly in a large codebase, where there could be more processing in between the call to `getUserNameAndEmail()` and the check on `userData.$1`, reviewers will not be able to tell immediately what is going on here.

Good ✅

```dart
Future<(String, String)> getUserNameAndEmail() async => return _someApiFetchMethod();
<Tabs>
<TabItem label="Good ✅">
```dart
Future<(String, String)> getUserNameAndEmail() async => return _someApiFetchMethod();
final (username, email) = await getUserNameAndEmail();
final (username, email) = await getUserNameAndEmail();
// a bunch of other code...
// a bunch of other code...
if (email.isValid) {
// do stuff
}
```
if (email.isValid) {
// do stuff
}
```
</TabItem>
</Tabs>

Now, we are expressly naming the values that we are getting from our record type. Any reviewer or future maintainer of this code will know what value is being validated.

Expand Down
50 changes: 23 additions & 27 deletions src/content/docs/error_handling/error_handling.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,31 @@ Properly documenting possible exceptions allows developers to handle exceptions,

<Tabs>
<TabItem label="Good ✅">

```dart
/// Permanently deletes an account with the given [name].
///
/// Throws:
///
/// * [UnauthorizedException] if the active role is not [Role.admin], since only
/// admins are authorized to delete accounts.
void deleteAccount(String name) {
if (activeRole != Role.admin) {
throw UnauthorizedException('Only admin can delete account');
}
// ...
}
```

```dart
/// Permanently deletes an account with the given [name].
///
/// Throws:
///
/// * [UnauthorizedException] if the active role is not [Role.admin], since only
/// admins are authorized to delete accounts.
void deleteAccount(String name) {
if (activeRole != Role.admin) {
throw UnauthorizedException('Only admin can delete account');
}
// ...
}
```
</TabItem>
<TabItem label="Bad ❗️">

```dart
/// Permanently deletes an account with the given [name].
void deleteAccount(String name) {
if (activeRole != Role.admin) {
throw UnauthorizedException('Only admin can delete account');
}
// ...
}
```

```dart
/// Permanently deletes an account with the given [name].
void deleteAccount(String name) {
if (activeRole != Role.admin) {
throw UnauthorizedException('Only admin can delete account');
}
// ...
}
```
</TabItem>
</Tabs>

Expand Down
Loading

0 comments on commit 10f8cd8

Please sign in to comment.