Skip to content

Few more Java 17 syntax improvements. #697

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

Merged
merged 2 commits into from
Feb 12, 2023
Merged

Conversation

bodiam
Copy link
Contributor

@bodiam bodiam commented Feb 12, 2023

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Feb 12, 2023

Codecov Report

Merging #697 (01d5840) into main (ad9f575) will increase coverage by 0.01%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main     #697      +/-   ##
============================================
+ Coverage     92.50%   92.51%   +0.01%     
- Complexity     2603     2604       +1     
============================================
  Files           283      283              
  Lines          5295     5291       -4     
  Branches        568      565       -3     
============================================
- Hits           4898     4895       -3     
  Misses          251      251              
+ Partials        146      145       -1     
Impacted Files Coverage Δ
.../datafaker/transformations/sql/SqlTransformer.java 86.23% <ø> (+0.31%) ⬆️
src/main/java/net/datafaker/formats/Xml.java 100.00% <100.00%> (ø)
...tafaker/idnumbers/pt/br/DocumentFormatterUtil.java 100.00% <100.00%> (ø)
...ker/idnumbers/pt/br/IdNumberGeneratorPtBrUtil.java 96.22% <100.00%> (+3.63%) ⬆️
...ain/java/net/datafaker/providers/base/Twitter.java 86.15% <100.00%> (+1.53%) ⬆️
.../main/java/net/datafaker/service/files/EnFile.java 93.33% <100.00%> (ø)
.../net/datafaker/transformations/XmlTransformer.java 91.76% <100.00%> (-0.29%) ⬇️
src/main/java/net/datafaker/formats/Json.java 84.00% <0.00%> (-2.29%) ⬇️
...net/datafaker/transformations/JsonTransformer.java 94.91% <0.00%> (+0.17%) ⬆️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 143 to 145
appender.append(String.valueOf(pad).repeat(Math.max(0, length - string.length())));
appender.append(string);
return appender.toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for StringBuilder here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not? What's the alternative?

Copy link
Collaborator

@snuyanzin snuyanzin Feb 12, 2023

Choose a reason for hiding this comment

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

return String.valueOf(pad).repeat(Math.max(0, length - string.length())) + string;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, like that. Yes, yes, can do.

@@ -484,7 +484,6 @@ public String getValue(Case caze) {
case UPPERCASE -> upperCaseValue;
case LOWERCASE -> lowerCaseValue;
case CAPITAL -> capitalValue;
default -> throw new IllegalArgumentException("Unknown case " + caze);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens if someone adds a new value to enum and forgets to update this switch?
With default exception the problem could be detected immediately.
How to detect it without default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it won't compile:

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok then

return List.of(TestSpec.of(astralia::locations, "australia.locations"),
TestSpec.of(astralia::animals, "australia.animals"),
TestSpec.of(astralia::states, "australia.states"));
Australia australia = faker.australia();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ugh, sorry that was me

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 didn't check, I feared it was me :-)

@bodiam bodiam merged commit 9aaa8a5 into main Feb 12, 2023
@kingthorin kingthorin deleted the migrate_to_java17_part_two branch February 12, 2023 12:06
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.

4 participants