Skip to content

Move component configuration parameters to end of constructor #496

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Move component configuration parameters to end of constructor #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/src/components/apple_pay/model/apple_pay_component_configuration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import 'package:adyen_checkout/adyen_checkout.dart';
import 'package:adyen_checkout/src/common/model/base_configuration.dart';

final class ApplePayComponentConfiguration extends BaseConfiguration {
final ApplePayConfiguration applePayConfiguration;

ApplePayComponentConfiguration({
required super.environment,
required super.clientKey,
Expand All @@ -14,6 +12,8 @@ final class ApplePayComponentConfiguration extends BaseConfiguration {
super.analyticsOptions,
});

final ApplePayConfiguration applePayConfiguration;

@override
String toString() {
return 'ApplePayComponentConfiguration(applePayConfiguration: $applePayConfiguration)';
Expand Down
8 changes: 4 additions & 4 deletions lib/src/components/card/model/card_component_configuration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ import 'package:adyen_checkout/src/common/model/base_configuration.dart';
import 'package:adyen_checkout/src/common/model/payment_method_configurations/card_configuration.dart';

final class CardComponentConfiguration extends BaseConfiguration {
final CardConfiguration cardConfiguration;

CardComponentConfiguration({
required super.environment,
required super.clientKey,
required super.countryCode,
super.amount,
super.shopperLocale,
super.analyticsOptions,
CardConfiguration? cardConfiguration,
}) : cardConfiguration = cardConfiguration ?? const CardConfiguration();
this.cardConfiguration = const CardConfiguration(),
});

final CardConfiguration cardConfiguration;
Comment on lines 4 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

While I see the intent for consistency, moving field declarations after the constructor goes against the common Dart convention of declaring fields at the top of the class. The Effective Dart style guide recommends this for better readability, as it allows developers to see all of a class's fields in one place.1

The change to use a default value for the cardConfiguration parameter is a great improvement. I suggest we keep that part and move the field declaration back to the top of the class.

Suggested change
final class CardComponentConfiguration extends BaseConfiguration {
final CardConfiguration cardConfiguration;
CardComponentConfiguration({
required super.environment,
required super.clientKey,
required super.countryCode,
super.amount,
super.shopperLocale,
super.analyticsOptions,
CardConfiguration? cardConfiguration,
}) : cardConfiguration = cardConfiguration ?? const CardConfiguration();
this.cardConfiguration = const CardConfiguration(),
});
final CardConfiguration cardConfiguration;
final class CardComponentConfiguration extends BaseConfiguration {
final CardConfiguration cardConfiguration;
CardComponentConfiguration({
required super.environment,
required super.clientKey,
required super.countryCode,
super.amount,
super.shopperLocale,
super.analyticsOptions,
this.cardConfiguration = const CardConfiguration(),
});

Style Guide References

Footnotes

  1. The Effective Dart style guide recommends placing field declarations at the top of a class, before any constructors or methods, to improve readability by grouping all instance variables together. ↩

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored based on the official style guide https://dart.dev/tools/linter-rules/sort_constructors_first

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @RahmiTufanoglu,
thanks for your adjustment and the recommendation of rearranging the constructor. I found this guideline in the Flutter style guide. So indeed we should think about adjusting the entire project and adopt this guideline.


@override
String toString() {
Expand Down
4 changes: 2 additions & 2 deletions lib/src/components/google_pay/model/google_pay_component_configuration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import 'package:adyen_checkout/adyen_checkout.dart';
import 'package:adyen_checkout/src/common/model/base_configuration.dart';

final class GooglePayComponentConfiguration extends BaseConfiguration {
final GooglePayConfiguration googlePayConfiguration;

GooglePayComponentConfiguration({
required super.environment,
required super.clientKey,
Expand All @@ -14,6 +12,8 @@ final class GooglePayComponentConfiguration extends BaseConfiguration {
super.analyticsOptions,
});

final GooglePayConfiguration googlePayConfiguration;

@override
String toString() {
return 'GooglePayComponentConfiguration(googlePayConfiguration: $googlePayConfiguration)';
Expand Down
24 changes: 12 additions & 12 deletions lib/src/drop_in/model/drop_in_configuration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@ import 'package:adyen_checkout/src/common/model/payment_method_configurations/go
import 'package:adyen_checkout/src/common/model/payment_method_configurations/stored_payment_method_configuration.dart';

final class DropInConfiguration extends BaseConfiguration {
final CardConfiguration? cardConfiguration;
final ApplePayConfiguration? applePayConfiguration;
final GooglePayConfiguration? googlePayConfiguration;
final CashAppPayConfiguration? cashAppPayConfiguration;
final StoredPaymentMethodConfiguration? storedPaymentMethodConfiguration;
final bool skipListWhenSinglePaymentMethod;
final String? preselectedPaymentMethodTitle;
final Map<String, String>? paymentMethodNames;

DropInConfiguration({
required super.environment,
required super.clientKey,
Expand All @@ -28,11 +19,20 @@ final class DropInConfiguration extends BaseConfiguration {
this.cashAppPayConfiguration,
this.storedPaymentMethodConfiguration,
this.preselectedPaymentMethodTitle,
bool? skipListWhenSinglePaymentMethod,
this.skipListWhenSinglePaymentMethod = false,
AnalyticsOptions? analyticsOptions,
this.paymentMethodNames,
}) : skipListWhenSinglePaymentMethod =
skipListWhenSinglePaymentMethod ?? false;
});

final CardConfiguration? cardConfiguration;
final ApplePayConfiguration? applePayConfiguration;
final GooglePayConfiguration? googlePayConfiguration;
final CashAppPayConfiguration? cashAppPayConfiguration;
final StoredPaymentMethodConfiguration? storedPaymentMethodConfiguration;
final bool skipListWhenSinglePaymentMethod;
final String? preselectedPaymentMethodTitle;
final Map<String, String>? paymentMethodNames;


@override
String toString() {
Expand Down