Skip to content

extract duplicate methods "toJavaNames" to a single place#1336

Merged
kingthorin merged 4 commits intomainfrom
refactor/duplicate-code-java-names
Aug 23, 2024
Merged

extract duplicate methods "toJavaNames" to a single place#1336
kingthorin merged 4 commits intomainfrom
refactor/duplicate-code-java-names

Conversation

@asolntsev
Copy link
Collaborator

No description provided.

@asolntsev asolntsev self-assigned this Aug 23, 2024
@what-the-diff
Copy link

what-the-diff bot commented Aug 23, 2024

PR Summary

  • Introduced a New 'JavaNames' Class
    We've added a new file called JavaNames.java to assist with name conversions in Java. This class includes a method called toJavaNames that helps convert any string to a standard Java naming convention.

  • Refactored 'FakeValues' and 'FakeValuesService' Classes
    Changes have been made to the already existing FakeValues.java and FakeValuesService.java files. Instead of each having their own toJavaNames method, they now import it from the newly added JavaNames class, which simplifies the code and avoids duplicate methods.

  • Added Tests for the New 'JavaNames' Method
    For thoroughness, we've also introduced a testing file named JavaNamesTest.java. This includes various test cases aimed at verifying proper functioning of the toJavaNames method in the JavaNames class.

@codecov
Copy link

codecov bot commented Aug 23, 2024

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.22%. Comparing base (1f346c7) to head (e1aff15).
Report is 9 commits behind head on main.

Files Patch % Lines
.../java/net/datafaker/internal/helper/JavaNames.java 95.45% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1336      +/-   ##
============================================
+ Coverage     92.04%   92.22%   +0.18%     
+ Complexity     3155     3144      -11     
============================================
  Files           316      318       +2     
  Lines          6232     6189      -43     
  Branches        634      612      -22     
============================================
- Hits           5736     5708      -28     
  Misses          337      337              
+ Partials        159      144      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

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

Could you change the method names in the tests to proper Java camelCase instead of using underscores?

@asolntsev
Copy link
Collaborator Author

Could you change the method names in the tests to proper Java camelCase instead of using underscores?

Done.

P.S. In fact, camelCaseIsNotReallyGoodForDescribingTestCases.
In many project, we_often_use_underscored_in_test_names. They are much better readable.

Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

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

Assuming the tests pass or failure is unrelated I'm good with this.

Thanks for the contribution!

@kingthorin
Copy link
Collaborator

Failure seems unrelated:

2024-08-23T19:38:52.9884050Z ##[error]Exception in thread "Thread-13" java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because the return value of "net.datafaker.internal.helper.CopyOnWriteMap.getOrDefault(Object, Object)" is null
2024-08-23T19:38:52.9892720Z 	at net.datafaker.service.FakeValuesService.fetchObject(FakeValuesService.java:267)
2024-08-23T19:38:52.9893280Z 	at net.datafaker.service.FakeValuesService.safeFetch(FakeValuesService.java:206)
2024-08-23T19:38:52.9895030Z 	at net.datafaker.service.FakeValuesService.resolve(FakeValuesService.java:505)
2024-08-23T19:38:52.9895350Z 	at net.datafaker.service.FakeValuesService.resolve(FakeValuesService.java:492)
2024-08-23T19:38:52.9895870Z 	at net.datafaker.providers.base.AbstractProvider.resolve(AbstractProvider.java:20)
2024-08-23T19:38:52.9896140Z 	at net.datafaker.providers.base.Address.stateAbbr(Address.java:98)
2024-08-23T19:38:52.9896380Z 	at net.datafaker.Issue759Test.fakeSomeData(Issue759Test.java:32)
2024-08-23T19:38:52.9896840Z 	at net.datafaker.Issue759Test$WorkerThread.run(Issue759Test.java:25)

@asolntsev
Copy link
Collaborator Author

Failure seems unrelated:

Yes, it's unrelated. It's the same flaky test that I described in #1310

@kingthorin kingthorin merged commit 197ac5f into main Aug 23, 2024
@kingthorin kingthorin deleted the refactor/duplicate-code-java-names branch August 23, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants