Add services layer, tests, streaming UI, and cleanup legacy code
This commit is contained in:
parent
5514fa6381
commit
d205d15c74
62 changed files with 3729 additions and 1024 deletions
|
|
@ -0,0 +1,132 @@
|
|||
---
|
||||
name: python-parentheses-comparison-bug
|
||||
description: |
|
||||
Debug Python comparison bug where parentheses around a variable cause unexpected behavior.
|
||||
Use when: (1) condition always evaluates to False/True unexpectedly, (2) code like
|
||||
"if (mylist) == 0" never triggers, (3) length check seems to not work, (4) comparison
|
||||
with list/dict returns unexpected results. Common mistake where parentheses cause the
|
||||
variable itself to be compared instead of its length.
|
||||
author: Claude Code
|
||||
version: 1.0.0
|
||||
date: 2026-01-31
|
||||
---
|
||||
|
||||
# Python Parentheses Comparison Bug
|
||||
|
||||
## Problem
|
||||
A subtle Python bug where unnecessary parentheses around a variable in a comparison
|
||||
cause the wrong value to be compared. The expression `(mylist) == 0` compares the list
|
||||
itself to 0, not its length. Since a list is never equal to an integer, this always
|
||||
returns False.
|
||||
|
||||
## Context / Trigger Conditions
|
||||
- Condition that should sometimes be True is always False (or vice versa)
|
||||
- Code pattern like `if (existing_items) == 0:` or `if (result) == expected:`
|
||||
- The parentheses don't cause a syntax error but change semantics
|
||||
- Often appears when copying/adapting code or during refactoring
|
||||
- May pass code review because it "looks" correct
|
||||
|
||||
## Solution
|
||||
|
||||
### Identify the Bug Pattern
|
||||
```python
|
||||
# BUG: Compares list to 0, always False
|
||||
if (existing_listings) == 0:
|
||||
return True
|
||||
|
||||
# Also wrong: compares list to integer
|
||||
if (items) == 5:
|
||||
do_something()
|
||||
```
|
||||
|
||||
### Fix: Use len() for Length Comparisons
|
||||
```python
|
||||
# CORRECT: Compares length to 0
|
||||
if len(existing_listings) == 0:
|
||||
return True
|
||||
|
||||
# Alternative: Use truthiness for empty check
|
||||
if not existing_listings:
|
||||
return True
|
||||
|
||||
# CORRECT: Compares length to integer
|
||||
if len(items) == 5:
|
||||
do_something()
|
||||
```
|
||||
|
||||
## Verification
|
||||
1. Add a debug print before the condition: `print(f"list={existing_listings}, len={len(existing_listings)}")`
|
||||
2. Verify the condition now evaluates correctly
|
||||
3. Write a unit test that exercises both branches of the condition
|
||||
|
||||
## Example
|
||||
|
||||
### Before (Broken)
|
||||
```python
|
||||
class FetchListingDetailsStep:
|
||||
async def needs_processing(self, listing_id: int) -> bool:
|
||||
existing_listings = await self.listing_repository.get_listings(
|
||||
only_ids=[listing_id]
|
||||
)
|
||||
# BUG: This compares the list object to 0, which is always False
|
||||
# The parentheses around existing_listings are misleading
|
||||
if (existing_listings) == 0:
|
||||
return True
|
||||
return False
|
||||
```
|
||||
|
||||
### After (Fixed)
|
||||
```python
|
||||
class FetchListingDetailsStep:
|
||||
async def needs_processing(self, listing_id: int) -> bool:
|
||||
existing_listings = await self.listing_repository.get_listings(
|
||||
only_ids=[listing_id]
|
||||
)
|
||||
# CORRECT: Check if list is empty using len()
|
||||
if len(existing_listings) == 0:
|
||||
return True
|
||||
return False
|
||||
```
|
||||
|
||||
### Even Better (Pythonic)
|
||||
```python
|
||||
class FetchListingDetailsStep:
|
||||
async def needs_processing(self, listing_id: int) -> bool:
|
||||
existing_listings = await self.listing_repository.get_listings(
|
||||
only_ids=[listing_id]
|
||||
)
|
||||
# Most Pythonic: Use truthiness
|
||||
return not existing_listings
|
||||
```
|
||||
|
||||
## Notes
|
||||
- Python's truthiness: empty collections are falsy, non-empty are truthy
|
||||
- This bug is particularly insidious because:
|
||||
- It's syntactically valid
|
||||
- It doesn't raise an exception
|
||||
- The parentheses make it look intentional
|
||||
- Code review may miss it
|
||||
- Linters like pylint or flake8 won't catch this specific pattern
|
||||
- Type checkers like mypy may warn about comparing incompatible types
|
||||
- When debugging, add print statements to verify actual vs expected values
|
||||
|
||||
## Prevention
|
||||
- Prefer `if not mylist:` over `if len(mylist) == 0:`
|
||||
- Prefer `if mylist:` over `if len(mylist) > 0:`
|
||||
- Remove unnecessary parentheses around single variables
|
||||
- Enable mypy's strict mode which may catch type comparison issues
|
||||
- Write unit tests that exercise both branches of conditions
|
||||
|
||||
## Related Patterns
|
||||
```python
|
||||
# These are all wrong (comparing object to number):
|
||||
if (mydict) == 0: # Always False
|
||||
if (mylist) > 0: # TypeError in Python 3
|
||||
if (mystring) == 0: # Always False
|
||||
|
||||
# These are correct:
|
||||
if len(mydict) == 0: # True if empty
|
||||
if not mydict: # True if empty (preferred)
|
||||
if len(mylist) > 0: # True if non-empty
|
||||
if mylist: # True if non-empty (preferred)
|
||||
```
|
||||
Loading…
Add table
Add a link
Reference in a new issue