[java] stream api usage enhancement

[iampopovich]

User description

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.

Description

This pull request includes several changes to improve exception messages and modify collection handling in the Selenium Java codebase. The most important changes include updating exception messages to use this instead of toString() and replacing Collectors.toSet() with Collectors.toUnmodifiableSet().

Improvements to exception messages:

• java/src/org/openqa/selenium/By.java: Updated NoSuchElementException message to use this instead of toString() in the findElement method.
• java/src/org/openqa/selenium/support/pagefactory/ByAll.java: Updated NoSuchElementException message to use this instead of toString() in the findElement method.
• java/src/org/openqa/selenium/support/pagefactory/ByChained.java: Updated NoSuchElementException message to use this instead of toString() in the findElement method.

Changes to collection handling:

• java/src/org/openqa/selenium/JavascriptExecutor.java: Replaced Collectors.toSet() with Collectors.toUnmodifiableSet() in the getPinnedScripts method.

Minor cleanup:

• java/src/org/openqa/selenium/JavascriptExecutor.java: Removed unnecessary import for Collections.

Motivation and Context

I provide these minor changes to reduce compiler warnings and keep code cleaner and simplier

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

enhancement

---

Description

• Improved exception messages across several classes (By, ByAll, ByChained) by using this instead of toString().
• Enhanced collection handling in JavascriptExecutor by replacing Collectors.toSet() with Collectors.toUnmodifiableSet().
• Removed unnecessary import of Collections in JavascriptExecutor.

---

Changes walkthrough 📝

Relevant files

Enhancement

By.java Improve exception message in `By` class                                    java/src/org/openqa/selenium/By.java Updated exception message to use this instead of toString(). +1/-1      JavascriptExecutor.java Enhance collection handling in `JavascriptExecutor`            java/src/org/openqa/selenium/JavascriptExecutor.java Replaced Collectors.toSet() with Collectors.toUnmodifiableSet(). Removed unnecessary import of Collections. +3/-5      ByAll.java Improve exception message in `ByAll` class                              java/src/org/openqa/selenium/support/pagefactory/ByAll.java Updated exception message to use this instead of toString(). +1/-1      ByChained.java Improve exception message in `ByChained` class                      java/src/org/openqa/selenium/support/pagefactory/ByChained.java Updated exception message to use this instead of toString(). +1/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Performance Impact The change from Collectors.toSet() to Collectors.toUnmodifiableSet() might have performance implications. While it provides immutability, it could potentially be slower for large collections. This should be validated, especially if this method is called frequently.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Utilize Java 8 streams and Optional for more concise and potentially safer element finding logic Consider using Java 8's Optional to handle the case when no element is found, which can make the code more expressive and potentially safer. java/src/org/openqa/selenium/support/pagefactory/ByAll.java [51-57] -for (By by : bys) { - List<WebElement> elements = context.findElements(by); - if (!elements.isEmpty()) { - return elements.get(0); - } -} -throw new NoSuchElementException("Cannot locate an element using " + this); +return Arrays.stream(bys) + .map(context::findElements) + .flatMap(List::stream) + .findFirst() + .orElseThrow(() -> new NoSuchElementException("Cannot locate an element using " + this)); Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion significantly enhances the code by using Java 8 streams and Optional, making the logic more concise and expressive. It also potentially increases safety by handling the absence of elements more gracefully.	8
	Simplify the stream operation by using a method reference instead of a lambda expression Consider using a method reference instead of a lambda expression for the map operation. This can make the code more concise and potentially easier to read. java/src/org/openqa/selenium/JavascriptExecutor.java [179-181] return UnpinnedScriptKey.getPinnedScripts(this).stream() - .map(key -> (ScriptKey) key) + .map(ScriptKey.class::cast) .collect(Collectors.toUnmodifiableSet()); Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to use a method reference instead of a lambda expression improves code readability and conciseness without altering functionality. This is a good practice in Java, making the code more idiomatic and easier to maintain.	7
Possible issue	Add a safeguard against potential null or empty list access Consider adding a null check for the allElements list before accessing its first element to prevent potential NullPointerException. java/src/org/openqa/selenium/By.java [123-126] if (allElements == null || allElements.isEmpty()) { throw new NoSuchElementException("Cannot locate an element using " + this); } -return allElements.get(0); +return allElements.stream().findFirst().orElseThrow(() -> new NoSuchElementException("Element list is empty")); Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion to use a stream operation with orElseThrow adds an additional safeguard against null or empty list access, which can prevent runtime exceptions and improve code robustness.	6
Best practice	Improve code structure by using an early return pattern and adding braces for clarity Consider using an early return statement to improve code readability and reduce nesting. java/src/org/openqa/selenium/support/pagefactory/ByChained.java [52-56] public WebElement findElement(SearchContext context) { List<WebElement> elements = findElements(context); - if (elements.isEmpty()) + if (elements.isEmpty()) { throw new NoSuchElementException("Cannot locate an element using " + this); + } return elements.get(0); } Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion to add braces improves code readability and reduces potential errors related to missing braces in future modifications. However, the early return pattern is already effectively used, so the impact is moderate.	5

💡 Need additional feedback ? start a PR chat

[pujagani]

LGTM. Thank you @iampopovich