-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: The comparator is greater than or less than the supported string comparison #4081
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,4 +21,8 @@ def compare(self, source_value, compare, target_value): | |
| try: | ||
| return float(source_value) > float(target_value) | ||
| except Exception as e: | ||
| try: | ||
| return str(source_value) > str(target_value) | ||
| except Exception as _: | ||
| pass | ||
| return False | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The provided code has some improvements that can be made for better robustness and readability:
Here’s an optimized version of the code: def compare(self, source_value, operator, target_value):
# Convert inputs to string
source_str = str(source_value).strip()
target_str = str(target_value).strip()
# Handle different operators: '>'
if operator == '>' and (source_str.isdigit() or target_str.isdigit()):
return source_str.isdigit() and int(source_str) > int(target_str)
return None # Return None for unsupported operationsExplanation:
This revised version makes the intent clear and reduces redundancy in error handling and conversion logic. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,4 +21,8 @@ def compare(self, source_value, compare, target_value): | |
| try: | ||
| return float(source_value) <= float(target_value) | ||
| except Exception as e: | ||
| try: | ||
| return str(source_value) <= str(target_value) | ||
| except Exception as _: | ||
| pass | ||
| return False | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The provided code has several issues that need to be addressed:
Here's an optimized version of the code without repetition and improved exception handling: def compare(self, source_value, operator, target_value):
comparisons = {
"<=": lambda x, y: self._compare(x, operator, y),
">": lambda x, y: self._compare(x, operator, y),
"==": lambda x, y: self._compare(x, operator, y),
"!=": lambda x, y: self._compare(x, operator, y),
">=": lambda x, y: self._compare(x, operator, y)
}
comparator = comparisons.get(operator)
return comparator(source_value, target_value) if comparator else False
def _compare(self, value1, symbol, value2):
try:
return eval(f"{value1} {symbol} {value2}")
except (TypeError, ValueError) as e:
print(f"Comparison failed with error: {e}")
return FalseKey Changes:
|
||
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.
The provided code has a few improvements to make it more robust:
Error Handling: The catch-all clause in the first exception handling block (
except Exception as e:) can be improved by using more specific exceptions instead ofException. This makes the error messages more informative and easier to debug.Default Return Value: The default return value is set to
Falsewhen an exceptio occurs during both string conversions. If you want a different behavior (e.g., logging or raising an custom exception), you might consider changing this.Performance Consideration: Converting strings to floats multiple times within a single operation can be inefficient, especially if performance is critical. You should ensure that the conversion from source to target values only happens once before comparison.
Here's the optimized version of the function considering these points:
Key Changes Made:
ExceptiontoValueErrorwithin the first try-except block to handle conversion errors specifically related to converting numeric values to floats.str.upper()) to perform natural order string comparison while ignoring case differences.