[py][bidi]: add `unhandled_prompt_behavior` param for `create_user_context` by navin772 · Pull Request #16112 · SeleniumHQ/selenium
PR Code Suggestions ✨
Explore these optional code suggestions:
| Category | Suggestion | Impact |
| General |
✅ Simplify dictionary building logic
Suggestion Impact:The suggestion was directly implemented - the repetitive if statements were replaced with a field_mapping dictionary and a loop that uses getattr() to dynamically access attributes and build the result dictionary
code diff:
+ field_mapping = {
+ "alert": "alert",
+ "before_unload": "beforeUnload",
+ "confirm": "confirm",
+ "default": "default",
+ "file": "file",
+ "prompt": "prompt",
+ }
+
result = {}
- if self.alert is not None:
- result["alert"] = self.alert
- if self.before_unload is not None:
- result["beforeUnload"] = self.before_unload
- if self.confirm is not None:
- result["confirm"] = self.confirm
- if self.default is not None:
- result["default"] = self.default
- if self.file is not None:
- result["file"] = self.file
- if self.prompt is not None:
- result["prompt"] = self.prompt
+ for attr_name, dict_key in field_mapping.items():
+ value = getattr(self, attr_name)
+ if value is not None:
+ result[dict_key] = value
The method manually checks each attribute and builds the dictionary, which is
repetitive and error-prone. Use a mapping approach to dynamically build the
result dictionary from instance attributes.
py/selenium/webdriver/common/bidi/session.py [80-100]
def to_dict(self) -> Dict[str, str]:
"""Convert the UserPromptHandler to a dictionary for BiDi protocol.
Returns:
-------
Dict[str, str]: Dictionary representation suitable for BiDi protocol
"""
+ field_mapping = {
+ 'alert': 'alert',
+ 'before_unload': 'beforeUnload',
+ 'confirm': 'confirm',
+ 'default': 'default',
+ 'file': 'file',
+ 'prompt': 'prompt',
+ }
+
result = {}
- if self.alert is not None:
- result["alert"] = self.alert
- if self.before_unload is not None:
- result["beforeUnload"] = self.before_unload
- if self.confirm is not None:
- result["confirm"] = self.confirm
- if self.default is not None:
- result["default"] = self.default
- if self.file is not None:
- result["file"] = self.file
- if self.prompt is not None:
- result["prompt"] = self.prompt
+ for attr_name, dict_key in field_mapping.items():
+ value = getattr(self, attr_name)
+ if value is not None:
+ result[dict_key] = value
return result[Suggestion processed]
Suggestion importance[1-10]: 5
__
Why: This suggestion improves maintainability by replacing a series of if statements with a more scalable, loop-based approach, making it easier to add new fields in the future.
| Low
|
Use dynamic parameter validation
The validation loop iterates over hardcoded field names and values, making it
fragile to changes. Use locals() or instance variables to dynamically validate
all parameters without hardcoding field names.
py/selenium/webdriver/common/bidi/session.py [60-71]
-for field_name, value in [
- ("alert", alert),
- ("before_unload", before_unload),
- ("confirm", confirm),
- ("default", default),
- ("file", file),
- ("prompt", prompt),
-]:
+params = {
+ 'alert': alert,
+ 'before_unload': before_unload,
+ 'confirm': confirm,
+ 'default': default,
+ 'file': file,
+ 'prompt': prompt,
+}
+
+for field_name, value in params.items():
if value is not None and value not in UserPromptHandlerType.VALID_TYPES:
raise ValueError(
f"Invalid {field_name} handler type: {value}. Must be one of {UserPromptHandlerType.VALID_TYPES}"
)Suggestion importance[1-10]: 3
__
Why: The suggestion offers a minor stylistic improvement by separating data from logic, but the proposed code is functionally equivalent and no less "fragile" than the original.
| Low
|
|
| |