Replacesd magic numbers with constants
This commit is contained in:
226
MAGIC_NUMBERS_ANALYSIS.md
Normal file
226
MAGIC_NUMBERS_ANALYSIS.md
Normal file
@@ -0,0 +1,226 @@
|
||||
# OcheCompanion - Magic Numbers and Hard-coded Values Analysis
|
||||
|
||||
## Summary
|
||||
|
||||
This document identifies magic numbers, hard-coded strings, and values that should be extracted as constants or resources.
|
||||
|
||||
## ✅ Completed: Utils Package Created
|
||||
|
||||
### New Files Created:
|
||||
1. **`utils/DartsConstants.java`** - Game logic constants (scores, dart values, multipliers, checkout limits)
|
||||
2. **`utils/UIConstants.java`** - UI constants (alphas, scales, durations, animations)
|
||||
3. **`utils/ResourceHelper.java`** - Helper methods for extracting colors, drawables, and strings
|
||||
4. **`utils/CheckoutConstants.java`** - Pre-calculated checkout routes for standard finishes
|
||||
|
||||
---
|
||||
|
||||
## 🔴 High Priority: Values to Replace
|
||||
|
||||
### GameActivity.java
|
||||
|
||||
#### Magic Numbers to Replace:
|
||||
| Current Code | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `mStartingScore = 501` | `DartsConstants.DEFAULT_GAME_SCORE` | Line 60 |
|
||||
| `getIntent().getIntExtra(EXTRA_START_SCORE, 501)` | `DartsConstants.DEFAULT_GAME_SCORE` | Line 165 |
|
||||
| `baseValue == 25` | `DartsConstants.BULL_VALUE` | Line 253 |
|
||||
| `mMultiplier == 3` | `DartsConstants.MULTIPLIER_TRIPLE` | Line 253 |
|
||||
| `points = 50` | `DartsConstants.DOUBLE_BULL_VALUE` | Line 253 |
|
||||
| `mMultiplier == 2` | `DartsConstants.MULTIPLIER_DOUBLE` | Line 260 |
|
||||
| `points == 50` | `DartsConstants.DOUBLE_BULL_VALUE` | Line 260 |
|
||||
| `onNumberTap(25)` | `DartsConstants.BULL_VALUE` | Line 296 |
|
||||
| `score <= 170` | `DartsConstants.MAX_CHECKOUT_SCORE` | Line 444 |
|
||||
| `score > 1` | `> DartsConstants.BUST_SCORE` | Line 444 |
|
||||
|
||||
#### Alpha/Opacity Values:
|
||||
| Current | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `1.0f` (alpha full) | `UIConstants.ALPHA_FULL` | Lines 306-308 |
|
||||
| `0.4f` (alpha inactive) | `UIConstants.ALPHA_INACTIVE` | Lines 306-308 |
|
||||
| `0.5f` (pulse min) | `UIConstants.ALPHA_PULSE_MIN` | Line 451 |
|
||||
| `1.0f` (pulse max) | `UIConstants.ALPHA_PULSE_MAX` | Line 451 |
|
||||
| `1000` (duration) | `UIConstants.ANIMATION_PULSE_DURATION` | Line 452 |
|
||||
|
||||
#### Hard-coded Strings:
|
||||
| Current | Suggested Constant/Resource | Location |
|
||||
|---|---|---|
|
||||
| `"DB"` | `DartsConstants.LABEL_DOUBLE_BULL` | Line 489 |
|
||||
| `"B"` | `DartsConstants.LABEL_BULL` | Line 490 |
|
||||
| `"BULL"` | `DartsConstants.LABEL_BULLSEYE` | Line 615 |
|
||||
| `"D"` + number | `DartsConstants.PREFIX_DOUBLE` | Line 614 |
|
||||
| `"T20"`, `"T19"`, `"D12"` | `CheckoutConstants.getCheckoutRoute()` | Lines 595-596 |
|
||||
| `" • "` (separator) | `DartsConstants.CHECKOUT_SEPARATOR` | Line 629 |
|
||||
| `"T20 Route"` | `DartsConstants.LABEL_T20_ROUTE` | Line 633 |
|
||||
| `" WINS!"` | String resource `R.string.player_wins_format` | Line 500 |
|
||||
|
||||
#### CheckoutEngine Values:
|
||||
| Current | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `score <= 40` | `DartsConstants.MAX_DIRECT_DOUBLE` | Line 614 |
|
||||
| `score == 50` | `DartsConstants.DOUBLE_BULL_VALUE` | Line 615 |
|
||||
| `score - 32` | `DartsConstants.SETUP_DOUBLE_32` | Line 623 |
|
||||
| `score - 40` | `DartsConstants.SETUP_DOUBLE_40` | Line 624 |
|
||||
| `score > 60` | `DartsConstants.HIGH_SCORE_THRESHOLD` | Line 633 |
|
||||
| Checkout map entries | `CheckoutConstants.getCheckoutRoute()` | Lines 595-602 |
|
||||
|
||||
---
|
||||
|
||||
### MainMenuActivity.java
|
||||
|
||||
#### Magic Numbers:
|
||||
| Current | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `501` | `DartsConstants.DEFAULT_GAME_SCORE` | Line 106 |
|
||||
| `testCounter % 3` | `UIConstants.TEST_CYCLE_MODULO` | Lines 156-165 |
|
||||
|
||||
#### Hard-coded Test Names:
|
||||
| Current | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `"Test1"` | `DartsConstants.TEST_PLAYER_1` | Lines 100, 156 |
|
||||
| `"Test2"` | `DartsConstants.TEST_PLAYER_2` | Lines 101, 157 |
|
||||
| `"Test3"` | `DartsConstants.TEST_PLAYER_3` | Line 158 |
|
||||
| `"Test4"` | `DartsConstants.TEST_PLAYER_4` | Line 159 |
|
||||
|
||||
---
|
||||
|
||||
### AddPlayerActivity.java
|
||||
|
||||
#### Scale/Zoom Values:
|
||||
| Current | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `1.0f` (normal scale) | `UIConstants.SCALE_NORMAL` | Lines 133, 262-264 |
|
||||
| `0.1f` (min zoom) | `UIConstants.SCALE_MIN_ZOOM` | Line 210 |
|
||||
| `10.0f` (max zoom) | `UIConstants.SCALE_MAX_ZOOM` | Line 210 |
|
||||
|
||||
#### Hard-coded Strings:
|
||||
| Current | Suggested Resource | Location |
|
||||
|---|---|---|
|
||||
| `"Image selected, entering Crop Mode: "` | `R.string.log_image_selected` | Line 137 |
|
||||
| `"AddPlayerActivity Created"` | `R.string.log_activity_created` | Line 149 |
|
||||
| `"image/*"` | String constant `MIME_TYPE_IMAGE` | Line 185 |
|
||||
|
||||
---
|
||||
|
||||
### CropOverlayView.java
|
||||
|
||||
#### Magic Number:
|
||||
| Current | Replace With | Location |
|
||||
|---|---|---|
|
||||
| `0.8f` (crop box ratio) | `UIConstants.CROP_BOX_SIZE_RATIO` | Line 60 |
|
||||
|
||||
---
|
||||
|
||||
### UI Adapters (MainMenuGroupMatchAdapter.java, MainMenuPlayerAdapter.java)
|
||||
|
||||
#### Drawable Resources Already Good:
|
||||
- `R.drawable.ic_users` - Already using resource reference ✓
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Medium Priority: Color Resources
|
||||
|
||||
### Already Using Resources (Good):
|
||||
```java
|
||||
R.color.volt_green
|
||||
R.color.triple_blue
|
||||
R.color.double_red
|
||||
R.color.text_primary
|
||||
R.color.border_subtle
|
||||
R.color.surface_primary
|
||||
```
|
||||
|
||||
### Suggested Enhancement:
|
||||
Use `ResourceHelper.getColor(context, R.color.xxx)` for consistency instead of direct `ContextCompat.getColor()` calls.
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Low Priority: Strings to Move to strings.xml
|
||||
|
||||
### Suggested String Resources:
|
||||
|
||||
```xml
|
||||
<resources>
|
||||
<!-- Game Messages -->
|
||||
<string name="player_wins_format">%s WINS!</string>
|
||||
<string name="player_busts">BUST!</string>
|
||||
|
||||
<!-- Checkout Labels -->
|
||||
<string name="label_double_bull">DB</string>
|
||||
<string name="label_bull">B</string>
|
||||
<string name="label_bullseye">BULL</string>
|
||||
<string name="label_t20_route">T20 Route</string>
|
||||
|
||||
<!-- Test Players -->
|
||||
<string name="test_player_1">Test1</string>
|
||||
<string name="test_player_2">Test2</string>
|
||||
<string name="test_player_3">Test3</string>
|
||||
<string name="test_player_4">Test4</string>
|
||||
|
||||
<!-- Logging -->
|
||||
<string name="log_image_selected">Image selected, entering Crop Mode: %s</string>
|
||||
<string name="log_activity_created">AddPlayerActivity Created</string>
|
||||
|
||||
<!-- Match Info -->
|
||||
<string name="match_recap_empty">No recent matches</string>
|
||||
<string name="player_average_format">AVG: %.1f</string>
|
||||
</resources>
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 📋 Implementation Checklist
|
||||
|
||||
### Step 1: Replace Game Constants in GameActivity
|
||||
- [ ] Replace `501` with `DartsConstants.DEFAULT_GAME_SCORE`
|
||||
- [ ] Replace `25` with `DartsConstants.BULL_VALUE`
|
||||
- [ ] Replace `50` with `DartsConstants.DOUBLE_BULL_VALUE`
|
||||
- [ ] Replace `170` with `DartsConstants.MAX_CHECKOUT_SCORE`
|
||||
- [ ] Replace multiplier checks with `MULTIPLIER_*` constants
|
||||
|
||||
### Step 2: Replace UI Constants
|
||||
- [ ] Replace alpha values with `UIConstants.ALPHA_*`
|
||||
- [ ] Replace scale values with `UIConstants.SCALE_*`
|
||||
- [ ] Replace animation duration with `UIConstants.ANIMATION_PULSE_DURATION`
|
||||
|
||||
### Step 3: Replace Hard-coded Strings in GameActivity
|
||||
- [ ] Replace dart labels with `DartsConstants.LABEL_*`
|
||||
- [ ] Replace checkout separator with `DartsConstants.CHECKOUT_SEPARATOR`
|
||||
- [ ] Move win message to string resource
|
||||
|
||||
### Step 4: Refactor CheckoutEngine
|
||||
- [ ] Use `CheckoutConstants.getCheckoutRoute()` instead of inline map
|
||||
- [ ] Replace checkout logic constants with `DartsConstants`
|
||||
|
||||
### Step 5: Update MainMenuActivity
|
||||
- [ ] Replace test player names with `DartsConstants.TEST_PLAYER_*`
|
||||
- [ ] Replace `501` with `DEFAULT_GAME_SCORE`
|
||||
- [ ] Replace modulo `3` with `TEST_CYCLE_MODULO`
|
||||
|
||||
### Step 6: Update AddPlayerActivity
|
||||
- [ ] Replace scale constants with `UIConstants.SCALE_*`
|
||||
|
||||
### Step 7: Update CropOverlayView
|
||||
- [ ] Replace `0.8f` with `UIConstants.CROP_BOX_SIZE_RATIO`
|
||||
|
||||
### Step 8: (Optional) Use ResourceHelper
|
||||
- [ ] Replace `ContextCompat.getColor()` with `ResourceHelper.getColor()`
|
||||
- [ ] Replace `ContextCompat.getDrawable()` with `ResourceHelper.getDrawable()`
|
||||
|
||||
---
|
||||
|
||||
## Benefits of These Changes
|
||||
|
||||
1. **Maintainability**: All game rules and values in one place
|
||||
2. **Consistency**: Same values used everywhere
|
||||
3. **Testability**: Easy to modify for testing different game modes
|
||||
4. **Localization**: String resources can be translated
|
||||
5. **Documentation**: Constants are self-documenting
|
||||
6. **Refactoring**: Change once, applies everywhere
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
- **Okay to Keep**: `0` and `1` in most cases (loop indices, null checks, etc.)
|
||||
- **Okay to Keep**: Resource IDs like `R.id.xxx`, `R.layout.xxx` (already constant)
|
||||
- **Consider**: Creating game mode enums (X01, Cricket, etc.) for future expansion
|
||||
Reference in New Issue
Block a user