Skip to content

EnvironmentType design tweaks#542

Merged
serhii-lekariev merged 21 commits intomasterfrom
error-prone-warnings
Jun 9, 2020
Merged

EnvironmentType design tweaks#542
serhii-lekariev merged 21 commits intomasterfrom
error-prone-warnings

Conversation

@serhii-lekariev
Copy link
Contributor

@serhii-lekariev serhii-lekariev commented Jun 9, 2020

This PR simplifies EnvironmentType and Environment.

Environment

Now, allows to check for the class with boolean is(Class<? extends EnvironmentType>), instead of boolean is(EnvironmentType)`.

EnvironmentType

Now, no longer overrides equals and hashCode in suspicious ways. Instead, it simply acts as a stricter interface for a single boolean enabled() method.

Also, standard environment types are no longer singletons. We no longer encourage the extenders to be singletons as well.

Also resolves some Error Prone warnings.

@serhii-lekariev serhii-lekariev marked this pull request as draft June 9, 2020 11:19
@serhii-lekariev serhii-lekariev self-assigned this Jun 9, 2020
@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #542 into master will decrease coverage by 0.00%.
The diff coverage is 83.33%.

@@             Coverage Diff              @@
##             master     #542      +/-   ##
============================================
- Coverage     73.79%   73.79%   -0.01%     
+ Complexity     2953     2951       -2     
============================================
  Files           506      506              
  Lines         11954    11943      -11     
  Branches        669      669              
============================================
- Hits           8822     8813       -9     
+ Misses         2904     2903       -1     
+ Partials        228      227       -1     

@serhii-lekariev serhii-lekariev marked this pull request as ready for review June 9, 2020 12:26
@serhii-lekariev
Copy link
Contributor Author

@dmdashenkov, PTAL.

}
public boolean is(Class<? extends EnvironmentType> typeClass) {
EnvironmentType currentType = cachedOrCalculated();
boolean result = currentType.getClass()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd go with type.isInstance(..). That will give us an opportunity to create polymorphic types.

}

// `Production` is the default fallback.
return new Production();
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like an illegal state to me.

@serhii-lekariev
Copy link
Contributor Author

@dmdashenkov, PTAL again.

@serhii-lekariev serhii-lekariev merged commit 31a6306 into master Jun 9, 2020
@serhii-lekariev serhii-lekariev deleted the error-prone-warnings branch June 9, 2020 13:35
@dmitrykuzmin dmitrykuzmin mentioned this pull request Sep 8, 2020
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