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

[#4209] refactor(DER): use StandardCharsets.UTF_8 instead of hardcoded "UTF-8" #4344

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mrkartik00
Copy link
Contributor

[#4209] refactor(DER): replace hardcoded string with StandardCharsets.UTF_8

Replaced hardcoded "UTF-8" with StandardCharsets.UTF_8 in DER.getAsString() method. This change improves code readability and aligns with modern Java practices for character encoding. No functional changes; only improved encoding method usage.

What changes were proposed in this pull request?

This pull request refactors the DER class to use StandardCharsets.UTF_8 instead of the hardcoded "UTF-8" string. The change specifically updates the DER.getAsString() method to improve code readability and align with modern Java practices for character encoding.

Why are the changes needed?

Readability: Using StandardCharsets.UTF_8 instead of a hardcoded string makes the code more readable and less prone to errors. It clearly indicates that UTF-8 encoding is used.

Consistency: Adhering to modern Java practices by using StandardCharsets ensures that the codebase is consistent with current standards and improves maintainability.
Fix: #4209

Does this PR introduce any user-facing change?

No, this pull request does not introduce any user-facing changes. The modifications are internal refactorings that do not alter any functionality or API behavior.

How was this patch tested?

Run Existing Unit Tests:
Executed all existing unit tests to confirm that the refactoring did not affect functionality. All tests passed successfully.

Manual Verification:
Checked that components using the DER class continue to work correctly.
Ensured that DER.getAsString() properly converts byte buffers to strings.

Code Review:
Reviewed the changes to verify correct usage of StandardCharsets.UTF_8 and ensure no unintended side effects.
No additional tests were needed as the changes were only refactoring and did not introduce new features or fix bugs.

…ing mechanism in DER class to use StandardCharsets.UTF_8 instead of the hardcoded "UTF-8" string. This change improves code readability and adheres to modern Java practices for character encoding. - Replaced hardcoded "UTF-8" with StandardCharsets.UTF_8 in DER.getAsString() method. - No functional changes; only improved encoding method usage.

This commit refactors the DER class to use `StandardCharsets.UTF_8` for character encoding. Previously, the code used a hardcoded string "UTF-8" for encoding and decoding purposes. By replacing this with `StandardCharsets.UTF_8`, we make the code more readable and consistent with modern Java standards.

The change was made in the `DER.getAsString()` method, where it converts a byte buffer to a string. This update does not introduce any functional changes to the code but aligns with best practices for encoding in Java.

Benefits of this change:
- **Readability**: `StandardCharsets.UTF_8` is a clearer representation of the UTF-8 charset and avoids potential typos with string literals.
- **Consistency**: Using `StandardCharsets` is a standard practice in Java for specifying character sets, making the codebase more maintainable.

No other parts of the code are affected by this change, ensuring that the functionality remains intact.
@FANNG1
Copy link
Contributor

FANNG1 commented Aug 5, 2024

Hi, @mrkartik00 , thanks for your PR, some other codes are using UTF-8 directly like HTTPClient.java etcs, could you fix them all?

Copy link
Member

@justinmclean justinmclean left a comment

Choose a reason for hiding this comment

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

No need for the updated comments, please remove them.

@mrkartik00
Copy link
Contributor Author

@FANNG1 Done the changes in HTTPClient.java you can check

@FANNG1
Copy link
Contributor

FANNG1 commented Aug 7, 2024

could you fix it in one PR? "UTF-8" exists in .//integration-test/src/test/java/org/apache/gravitino/integration/test/store/relational/service/FilesetMetaServiceIT.java and .//integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/AbstractIT.java too.

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.

[Improvement] In KerberosServerUtils.java replace hardcoded string
3 participants