-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
[java] Refactored the UsernameAndPassword
class to a record
#15360
base: trunk
Are you sure you want to change the base?
Conversation
Refactored the `UsernameAndPassword` class to a `record`, simplifying the implementation and ensuring immutability. Removed the redundant `of` method and `Supplier<Credentials>` usage, as they are no longer needed with the record structure.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
UsernameAndPassword
class to a record
@manuelsblanco Can you resolve the conflict? |
|
||
public UsernameAndPassword(String username, String password) { | ||
this.username = Require.nonNull("User name", username); | ||
this.password = Require.nonNull("Password", password); | ||
} | ||
|
||
public static Supplier<Credentials> of(String username, String password) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, note that this method is used by Selenium users, for example:
import org.openqa.selenium.UsernameAndPassword;
import org.openqa.selenium.chrome.ChromeDriver;
public class Main {
public static void main(String[] args) throws InterruptedException {
var driver = new ChromeDriver();
driver.register(UsernameAndPassword.of("foo", "bar"));
driver.get("http://httpbin.org/basic-auth/foo/bar");
Thread.sleep(10_000);
driver.quit();
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mk868 New commit with the new code added. Thanks
Sure, I will try this wkd. |
- Added the static factory method `of(String username, String password)` to maintain compatibility with existing Selenium code that relies on it. - Optimized the constructor by using a compact form, ensuring `username` and `password` are non-null without explicitly redefining fields. - Removed redundant `username()` and `password()` methods, as they are automatically generated by the `record` structure. - Improved code clarity, maintainability, and adherence to Java's record best practices.
This is ready to check @VietND96 |
Not sure why the PR still showing branch has conflicts that must be resolved |
Fixed @VietND96 |
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Delta456 newline added
does this look ok? @pujagani If we want to update the code, we have several classes that could be converted into record classes. |
User description
Refactored the
UsernameAndPassword
class to arecord
, simplifying the implementation and ensuring immutability. Removed the redundantof
method andSupplier<Credentials>
usage, as they are no longer needed with the record structure.Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Motivation and Context
Types of changes
Checklist
PR Type
Enhancement
Description
Refactored
UsernameAndPassword
class into a Javarecord
.Simplified implementation by removing redundant methods.
Ensured immutability and streamlined the code structure.
Changes walkthrough 📝
UsernameAndPassword.java
Refactored `UsernameAndPassword` class to a `record`
java/src/org/openqa/selenium/UsernameAndPassword.java
UsernameAndPassword
class to a Javarecord
.of
method andSupplier
usage.username
andpassword
.Require.nonNull
checks.