Browse Source
- Fix DetectionController initialization crash by properly setting up dependency injection - Update all function signatures to use new MLDetection type instead of legacy Detection - Remove duplicate class mapping and getClassIdFromName helper method from ScreenCaptureService - Update EnhancedOCR to work with new MLDetection and BoundingBox types - Comment out legacy YOLOOnnxDetector references in MainActivity and DetectionController - Exclude legacy YOLOOnnxDetector.kt from compilation (moved to .bak) - Fix missing return statement in YOLOInferenceEngine.detectWithMat method - Add build commands documentation to CLAUDE.md for future reference - Ensure all preprocessing and detection functionality preserved from original Architecture Changes: - ScreenCaptureService now properly uses MLInferenceEngine interface - DetectionController updated to use MLInferenceEngine instead of legacy detector - All detection workflows now use unified MLDetection and BoundingBox types - Maintained 100% backward compatibility of ML detection capabilities 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>arch-002-ml-inference-engine
8 changed files with 289 additions and 203 deletions
@ -0,0 +1,192 @@ |
|||||
|
Code Cleanup Report: YOLOInferenceEngine.kt |
||||
|
|
||||
|
Overview |
||||
|
|
||||
|
The YOLOInferenceEngine.kt file successfully preserves all original functionality from the legacy YOLOOnnxDetector, but contains several areas that |
||||
|
could benefit from cleanup and modernization. This report identifies opportunities for improvement while maintaining the robust feature set. |
||||
|
|
||||
|
1. Architecture & Design Issues |
||||
|
|
||||
|
1.1 Mixed Responsibilities |
||||
|
|
||||
|
- Issue: The class handles both high-level inference coordination and low-level OpenCV operations |
||||
|
- Impact: Violates Single Responsibility Principle, makes testing difficult |
||||
|
- Recommendation: Extract OpenCV preprocessing into separate ImagePreprocessor class (partially done but could be expanded) |
||||
|
|
||||
|
1.2 Static Configuration |
||||
|
|
||||
|
companion object { |
||||
|
// Multiple preprocessing techniques |
||||
|
private const val ENABLE_MULTI_PREPROCESSING = true |
||||
|
private const val ENABLE_ENHANCED_PREPROCESSING = true |
||||
|
private const val ENABLE_SHARPENED_PREPROCESSING = true |
||||
|
} |
||||
|
- Issue: Hard-coded feature flags prevent runtime configuration |
||||
|
- Recommendation: Move to constructor parameters or configuration class |
||||
|
|
||||
|
2. Code Duplication & Redundancy |
||||
|
|
||||
|
2.1 Duplicate Class Mapping |
||||
|
|
||||
|
- Issue: 96-class mapping is duplicated from original detector |
||||
|
- Location: Lines 52-149 (classNames map) |
||||
|
- Recommendation: Extract to shared constants file or enum class |
||||
|
|
||||
|
2.2 Repeated Preprocessing Logic |
||||
|
|
||||
|
// Similar preprocessing code appears in multiple methods: |
||||
|
// - detectWithPreprocessing() |
||||
|
// - preprocessImageUltralytics() |
||||
|
// - preprocessImageEnhanced() |
||||
|
- Issue: Similar Mat operations repeated across methods |
||||
|
- Recommendation: Create common preprocessing pipeline with strategy pattern |
||||
|
|
||||
|
2.3 Coordinate Transformation Duplication |
||||
|
|
||||
|
- Issue: Letterbox calculations repeated in multiple coordinate modes |
||||
|
- Recommendation: Extract to utility methods |
||||
|
|
||||
|
3. Error Handling & Resource Management |
||||
|
|
||||
|
3.1 Inconsistent Error Handling |
||||
|
|
||||
|
// Some methods return null on error, others return empty lists |
||||
|
private fun preprocessImageUltralytics(inputMat: Mat): Mat? { /* returns null */ } |
||||
|
private suspend fun detectWithMat(inputMat: Mat): List<Detection> { /* returns emptyList() */ } |
||||
|
- Issue: Inconsistent error return patterns |
||||
|
- Recommendation: Standardize on Result or sealed class for error handling |
||||
|
|
||||
|
3.2 Resource Cleanup Patterns |
||||
|
|
||||
|
// Manual Mat cleanup scattered throughout |
||||
|
mat.release() |
||||
|
croppedMat?.release() |
||||
|
- Issue: Risk of memory leaks if exceptions occur before cleanup |
||||
|
- Recommendation: Use extension functions or try-with-resources pattern |
||||
|
|
||||
|
4. Performance & Concurrency Issues |
||||
|
|
||||
|
4.1 Thread Pool Management |
||||
|
|
||||
|
private suspend fun detectWithMultiplePreprocessing(inputMat: Mat): List<Detection> { |
||||
|
val executor = Executors.newFixedThreadPool(3) |
||||
|
// ... usage |
||||
|
executor.shutdown() |
||||
|
} |
||||
|
- Issue: Creating new thread pools for each detection call |
||||
|
- Recommendation: Use shared, properly managed executor or coroutines |
||||
|
|
||||
|
4.2 Synchronous Operations in Async Context |
||||
|
|
||||
|
- Issue: Blocking ONNX inference calls inside suspend functions |
||||
|
- Recommendation: Use withContext(Dispatchers.Default) for CPU-intensive work |
||||
|
|
||||
|
5. Code Organization & Readability |
||||
|
|
||||
|
5.1 Large Method Sizes |
||||
|
|
||||
|
- Issue: detectWithMat() method is ~40 lines with complex nested logic |
||||
|
- Recommendation: Break into smaller, focused methods |
||||
|
|
||||
|
5.2 Magic Numbers |
||||
|
|
||||
|
private const val IOU_THRESHOLD = 0.4f |
||||
|
private const val CONFIDENCE_THRESHOLD = 0.25f |
||||
|
private const val MAX_DETECTIONS = 8400 |
||||
|
- Issue: Some thresholds hard-coded, others configurable |
||||
|
- Recommendation: Make all thresholds configurable with sensible defaults |
||||
|
|
||||
|
5.3 Complex Conditional Logic |
||||
|
|
||||
|
val detections = if (ENABLE_MULTI_PREPROCESSING) { |
||||
|
// Multiple preprocessing methods with parallel execution |
||||
|
detectWithMultiplePreprocessing(inputMat) |
||||
|
} else { |
||||
|
// Single preprocessing method |
||||
|
detectWithPreprocessing(inputMat, "ultralytics") |
||||
|
} |
||||
|
- Recommendation: Use strategy pattern for preprocessing selection |
||||
|
|
||||
|
6. Testing & Maintainability |
||||
|
|
||||
|
6.1 Hard to Test Components |
||||
|
|
||||
|
- Issue: Direct ONNX runtime calls, OpenCV operations mixed with business logic |
||||
|
- Recommendation: Extract interfaces for ONNX operations to enable mocking |
||||
|
|
||||
|
6.2 Configuration Flexibility |
||||
|
|
||||
|
// Current: Hard-coded coordinate modes |
||||
|
when (coordinateMode) { |
||||
|
"HYBRID" -> // ... |
||||
|
"LETTERBOX" -> // ... |
||||
|
"DIRECT" -> // ... |
||||
|
} |
||||
|
- Issue: String-based configuration prone to typos |
||||
|
- Recommendation: Use enum classes for type safety |
||||
|
|
||||
|
7. Documentation & Comments |
||||
|
|
||||
|
7.1 Missing Method Documentation |
||||
|
|
||||
|
- Issue: Complex algorithms like weighted NMS lack comprehensive documentation |
||||
|
- Recommendation: Add KDoc comments explaining algorithm logic and parameters |
||||
|
|
||||
|
7.2 TODO Comments |
||||
|
|
||||
|
// Multiple TODO comments indicate incomplete features |
||||
|
- Issue: Technical debt markers need addressing |
||||
|
- Recommendation: Create issues for TODOs or implement missing features |
||||
|
|
||||
|
8. Modern Kotlin Opportunities |
||||
|
|
||||
|
8.1 Data Classes vs Manual Classes |
||||
|
|
||||
|
// Could use data classes for simple containers |
||||
|
class Detection(val className: String, val confidence: Float, val boundingBox: BoundingBox) |
||||
|
|
||||
|
8.2 Extension Functions |
||||
|
|
||||
|
// Repeated OpenCV operations could be extensions |
||||
|
fun Mat.letterboxResize(targetSize: Size): Mat { /* ... */ } |
||||
|
|
||||
|
8.3 Sealed Classes for State |
||||
|
|
||||
|
// For preprocessing results, detection states |
||||
|
sealed class PreprocessingResult { |
||||
|
data class Success(val mat: Mat) : PreprocessingResult() |
||||
|
data class Error(val message: String) : PreprocessingResult() |
||||
|
} |
||||
|
|
||||
|
9. Recommended Cleanup Priority |
||||
|
|
||||
|
High Priority (Functional Impact) |
||||
|
|
||||
|
1. Fix resource management patterns to prevent memory leaks |
||||
|
2. Standardize error handling approaches |
||||
|
3. Extract thread pool management to class level |
||||
|
|
||||
|
Medium Priority (Maintainability) |
||||
|
|
||||
|
1. Break down large methods |
||||
|
2. Extract common preprocessing logic |
||||
|
3. Move configuration to constructor/config class |
||||
|
|
||||
|
Low Priority (Code Quality) |
||||
|
|
||||
|
1. Add comprehensive documentation |
||||
|
2. Apply modern Kotlin patterns |
||||
|
3. Extract magic numbers to constants |
||||
|
|
||||
|
10. Estimated Effort |
||||
|
|
||||
|
- High Priority: 2-3 days |
||||
|
- Medium Priority: 3-4 days |
||||
|
- Low Priority: 2-3 days |
||||
|
- Total Estimated: 7-10 days for complete cleanup |
||||
|
|
||||
|
Conclusion |
||||
|
|
||||
|
The YOLOInferenceEngine.kt successfully maintains all original functionality while providing a clean interface. The identified cleanup opportunities |
||||
|
would improve maintainability, testability, and performance without affecting the robust ML detection capabilities. The code represents a solid |
||||
|
foundation that could benefit from gradual refactoring using the boy scout rule: "leave the code better than you found it." |
||||
Loading…
Reference in new issue