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

Default Attribute Value On Instance #26

Merged
merged 3 commits into from
Sep 25, 2022

Conversation

chandrabezzo
Copy link
Contributor

Make attributes nullable when instance SDK

@@ -8,15 +8,15 @@ abstract class SDKBuilder {
SDKBuilder({
required this.apiKey,
required this.hostURL,
required this.attributes,
this.attributes,
Copy link
Contributor

Choose a reason for hiding this comment

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

attributes should not be null. Instead, you can initialize this with a default value.

  SDKBuilder({
    required this.apiKey,
    required this.hostURL,
    this.attributes = const {},
  })

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 see in the super constructor is nullable, why attributes from SDKBuilder should not be null? What's the impact when we send null or empty map?

Copy link
Contributor

Choose a reason for hiding this comment

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

The core FeatureEvaluator evaluates the condition against the attributes and the condition. If attributes are null then we can not evaluate the condition. It's better to have empty in place of null attributes.

make sure it's formatted before you push code. You can ensure it by flutter format .. You can fire this from root.

LGTM.

@chandrabezzo chandrabezzo changed the title Nullable Attribute Instance Default Attribute Value On Instance Sep 23, 2022
@DK070202 DK070202 merged commit cfa313f into alippo-com:master Sep 25, 2022
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.

None yet

2 participants