-
Notifications
You must be signed in to change notification settings - Fork 0
Create codereview-test #17
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env python3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| A simple calculator script with various errors for testing code review tools. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json # unused import | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def calculate_average(numbers): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Missing input validation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| total = 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for num in numbers: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| total += num | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Division by zero potential | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return total / len(numbers) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+16
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. Add input validation to prevent division by zero. The function will raise a Apply this diff to fix the issues: -def calculate_average(numbers):
- # Missing input validation
- total = 0
- for num in numbers:
- total += num
- # Division by zero potential
- return total / len(numbers)
+def calculate_average(numbers: list[float]) -> float:
+ """Calculate the average of a list of numbers.
+
+ Args:
+ numbers: List of numeric values
+
+ Returns:
+ The average of the numbers
+
+ Raises:
+ ValueError: If the list is empty
+ """
+ if not numbers:
+ raise ValueError("Cannot calculate average of empty list")
+ return sum(numbers) / len(numbers)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def process_data(data): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Undefined variable error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for item in data: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if item > threshold: # threshold is not defined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result.append(item * 2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+24
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. Correctness: The 📝 Committable Code Suggestion
Suggested change
Comment on lines
+18
to
+24
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. Fix undefined variable error. The variable Apply this diff to fix the issue by adding -def process_data(data):
- # Undefined variable error
- result = []
- for item in data:
- if item > threshold: # threshold is not defined
- result.append(item * 2)
- return result
+def process_data(data: list[float], threshold: float) -> list[float]:
+ """Process data by filtering and transforming values above threshold.
+
+ Args:
+ data: List of numeric values
+ threshold: Minimum value to include
+
+ Returns:
+ List of values above threshold, multiplied by 2
+ """
+ return [item * 2 for item in data if item > threshold]📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class Calculator: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.history = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def add(self, a, b): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Missing type hints and validation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = a + b | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.history.append(result) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+30
to
+34
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. 🛠️ Refactor suggestion Add type hints and validation to the add method. The method lacks type hints which reduces code clarity and maintainability. Apply this diff to improve the method: - def add(self, a, b):
- # Missing type hints and validation
- result = a + b
- self.history.append(result)
- return result
+ def add(self, a: float, b: float) -> float:
+ """Add two numbers and store result in history.
+
+ Args:
+ a: First number
+ b: Second number
+
+ Returns:
+ Sum of a and b
+ """
+ result = a + b
+ self.history.append(result)
+ return result📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def divide(self, a, b): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # No zero division check | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return a / b | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+36
to
+38
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. Correctness: The 📝 Committable Code Suggestion
Suggested change
Comment on lines
+36
to
+38
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. Add zero division check. The divide method will raise a Apply this diff to add proper error handling: - def divide(self, a, b):
- # No zero division check
- return a / b
+ def divide(self, a: float, b: float) -> float:
+ """Divide two numbers.
+
+ Args:
+ a: Dividend
+ b: Divisor
+
+ Returns:
+ Result of a divided by b
+
+ Raises:
+ ValueError: If divisor is zero
+ """
+ if b == 0:
+ raise ValueError("Cannot divide by zero")
+ return a / b📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def get_history(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Returning mutable reference | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return self.history | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+42
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. Correctness: The 📝 Committable Code Suggestion
Suggested change
Comment on lines
+40
to
+42
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. 🛠️ Refactor suggestion Return a copy of history to prevent external modifications. Returning the mutable list directly allows external code to modify the internal state of the Calculator. Apply this diff to fix the encapsulation issue: - def get_history(self):
- # Returning mutable reference
- return self.history
+ def get_history(self) -> list[float]:
+ """Get a copy of the calculation history.
+
+ Returns:
+ Copy of the history list
+ """
+ return self.history.copy()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def main(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Syntax error - missing closing bracket | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| numbers = [1, 2, 3, 4, 5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+46
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. Fix syntax error - missing closing bracket. The list definition is missing a closing bracket, which will cause a Apply this diff: - # Syntax error - missing closing bracket
- numbers = [1, 2, 3, 4, 5
+ numbers = [1, 2, 3, 4, 5]📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+45
to
+46
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. Correctness: Syntax error in 📝 Committable Code Suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Logic error - wrong function name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| avg = calculate_mean(numbers) # function is called calculate_average | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Correctness: The function 📝 Committable Code Suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Average: {avg}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+50
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. Fix function name error. The function Apply this diff: - # Logic error - wrong function name
- avg = calculate_mean(numbers) # function is called calculate_average
- print(f"Average: {avg}")
+ avg = calculate_average(numbers)
+ print(f"Average: {avg}")📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Unused variable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| unused_var = "this is never used" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Inefficient loop | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| large_list = list(range(10000)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| squared = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i in range(len(large_list)): # should use enumerate or direct iteration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| squared.append(large_list[i] ** 2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # String concatenation in loop (inefficient) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message = "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i in range(100): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message += f"Item {i} " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Security issue - using eval | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_input = "2 + 2" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = eval(user_input) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+68
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. Remove eval() - serious security vulnerability. Using Apply this diff: - # Security issue - using eval
- user_input = "2 + 2"
- result = eval(user_input)
+ # Safe alternative to eval for simple arithmetic
+ result = 2 + 2If you need to evaluate mathematical expressions from user input, consider using a safe expression parser library like 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+66
to
+68
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. Security: Using 📝 Committable Code Suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Missing exception handling | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open("nonexistent_file.txt", "r") as f: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| content = f.read() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+70
to
+72
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. Add exception handling for file operations. File operations should always include proper exception handling to handle cases where the file doesn't exist or can't be read. Apply this diff: - # Missing exception handling
- with open("nonexistent_file.txt", "r") as f:
- content = f.read()
+ try:
+ with open("nonexistent_file.txt", "r") as f:
+ content = f.read()
+ except FileNotFoundError:
+ print("Error: File 'nonexistent_file.txt' not found")
+ content = ""
+ except IOError as e:
+ print(f"Error reading file: {e}")
+ content = ""📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+70
to
+72
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. Correctness: File operation lacks exception handling, which will cause the program to crash when trying to open a non-existent file. 📝 Committable Code Suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| calc = Calculator() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Potential error - passing string to add method | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| calc.add("5", 3) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+74
to
+76
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. Fix type error in Calculator usage. Passing a string Apply this diff: - calc = Calculator()
- # Potential error - passing string to add method
- calc.add("5", 3)
+ calc = Calculator()
+ calc.add(5, 3)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+74
to
+76
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. Correctness: The 📝 Committable Code Suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| main() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Correctness: The
calculate_averagefunction lacks input validation, which could lead to runtime errors ifnumbersis empty or contains non-numeric values.📝 Committable Code Suggestion