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

Refactoring - Implementation refactoring #739

Closed
wants to merge 10 commits into from
Closed

Refactoring - Implementation refactoring #739

wants to merge 10 commits into from

Conversation

almasfiza
Copy link
Contributor

This PR has three different implementation refactoring performed.

  1. Extract method refactoring in datafaker/src/main/java/net/datafaker/providers/base/Barcode.java (used enhanced switch as suggested in issue)
  2. Rename variable for better understanding in datafaker/src/main/java/net/datafaker/service/FakerIDN.java (renamed Stringbuilder sb to resultASCII since it stores the ASCII equivalent).
  3. Decompose conditional refactoring in src/main/java/net/datafaker/providers/base/Number.java ( creating a method isValidRange to return the result of the condition instead of using the long condition in the if statement directly).

@what-the-diff
Copy link

what-the-diff bot commented Mar 28, 2023

PR Summary

  • 🚀 Method Extraction
    Refactored the code by extracting calculateOutput method for better readability and organization.
  • 🔍 Range Validation
    Added private isValidRangeLong and isValidRange methods to check if the range of numbers in numberBetween are valid.
  • 💡 Modern Syntax
    Used the var keyword for better readability and simplicity in the FakerIDN class, replacing StringBuilder with String.

@bodiam
Copy link
Contributor

bodiam commented Mar 28, 2023

Strange PR. Is this a test of an AI refactoring or so?

@almasfiza
Copy link
Contributor Author

Strange PR. Is this a test of an AI refactoring or so?

No, I am working on an assignment, which is on refactoring and encourages us to contribute to open source repositories.

Co-authored-by: Sergey Nuyanzin <sergey.nuyanzin@aiven.io>
@almasfiza almasfiza requested a review from snuyanzin March 29, 2023 01:11
@bodiam
Copy link
Contributor

bodiam commented Mar 29, 2023

No, I am working on an assignment, which is on refactoring and encourages us to contribute to open source repositories.

Ah, that explains! I was surprised by the overly structured changeset, normally it's something like "changed a thing", and not used to such extended descriptions.

Thank you for picking this project for your assignment. Happy to integrate your contributions into this project!

@bodiam
Copy link
Contributor

bodiam commented Mar 29, 2023

(Though it seems the build is failing)

@almasfiza
Copy link
Contributor Author

(Though it seems the build is failing)

I will check on that and open a fresh PR. Any suggestions on how I should proceed?

@almasfiza almasfiza closed this by deleting the head repository Mar 29, 2023
kingthorin added a commit that referenced this pull request Mar 29, 2023
Refactor1 : Worked on the failed build for previous PR #739
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.

3 participants