claude-code-best-practice

Here's a review of a Python script that processes user uploads in a web application:

**Original Code:**
```python
def process_upload(file_path):
    with open(file_path, 'r') as f:
        data = f.read()
    lines = data.split('\n')
    for line in lines:
        if line.strip():
            process_line(line)
```

**Issues Identified:**

1. **Resource Management:** The file handle isn't explicitly closed (though the `with` statement handles this, it's good practice to be explicit). Consider using `try/finally` for more complex scenarios.

2. **Memory Efficiency:** Reading the entire file into memory with `f.read()` is problematic for large files. This could cause memory issues with files >100MB.

3. **Error Handling:** No try-catch block for file operations. What happens if the file doesn't exist or isn't readable?

4. **Documentation:** Missing docstring and type hints. Future maintainers won't know the expected input/output.

5. **Performance:** The `split('\n')` creates an unnecessary intermediate list. Consider iterating directly over the file object.

**Improved Version:**
```python
def process_upload(file_path: str) -> None:
    """Process a user upload file line by line with proper error handling.
    
    Args:
        file_path: Path to the file to process
        
    Raises:
        FileNotFoundError: If file doesn't exist
        PermissionError: If file isn't readable
        ValueError: If file is empty or invalid
    """
    try:
        with open(file_path, 'r') as f:
            for line in f:
                if line.strip():
                    process_line(line.strip())
    except FileNotFoundError as e:
        raise FileNotFoundError(f"Upload file not found: {file_path}") from e
    except PermissionError as e:
        raise PermissionError(f"No permission to read file: {file_path}") from e
    except Exception as e:
        raise ValueError(f"Error processing file {file_path}: {str(e)}") from e
```

**Additional Recommendations:**
- Consider adding file size validation before processing
- Implement logging for debugging and monitoring
- Add unit tests for edge cases (empty files, special characters)
- For production, consider using a streaming approach with `yield` for very large files
- Add input validation for the file_path parameter