-
-
Notifications
You must be signed in to change notification settings - Fork 122
Description
Summary
The current implementation of WeightScreen works functionally but contains multiple structural, reliability, and maintainability issues. These need to be addressed to align the component with production-grade standards and improve long-term scalability.
Areas of Concern
1. API Handling (High Priority)
Currently, all network requests are made using raw fetch inside the component:
GET /weightPOST /weightPUT /weight/:idDELETE /weight/:id
Problems:
- Repeated boilerplate code
- Inconsistent error handling
- No timeout handling
- No retry mechanism
- Harder debugging and maintenance
Proposed Fix:
Introduce a centralized API client (e.g., apiClient.js) and replace all direct fetch calls.
2. Missing Loading State (UX Issue)
There is no loading indicator when fetching weight history. If the API is slow, the user sees a blank screen with no feedback.
Expected Behavior:
A loader or skeleton should appear while data is being fetched.
3. Unsafe Date Handling
The function formatLocalDate manually appends "Z" to timestamps:
const dateStringWithZ = utcDateString.endsWith('Z')
? utcDateString
: `${utcDateString}Z`;This:
- Assumes backend always returns UTC
- May cause incorrect timestamps
- Can break if backend format changes
Proposed Fix:
Use a proper date library like dayjs or date-fns, or rely strictly on backend ISO format.
4. Insufficient Input Validation
Current validation only checks for empty fields:
if (!week || !weight) return;It does NOT prevent:
- Negative weeks
- Week > 40
- Negative or zero weight
- Non-numeric values
Expected Validation:
const weekNum = parseInt(week);
const weightNum = parseFloat(weight);
if (weekNum < 1 || weekNum > 40) {
Alert.alert("Invalid week");
return;
}
if (weightNum <= 0) {
Alert.alert("Invalid weight");
return;
}5. Unstable Keys in List Rendering
The history list currently uses index as key:
{history.map((entry, index) => (
<Card key={index}>This can cause incorrect UI behavior when items are edited or deleted.
Fix:
Use a stable unique key:
<Card key={entry.id}>6. State Mutation with reverse()
This line mutates the original array:
setHistory(data.reverse());Fix:
setHistory([...data].reverse());7. Potential Crash in Edit Flow
If editData is accessed before being set, it could cause runtime errors.
Safer initialization:
const [editData, setEditData] = useState({
id: null,
week_number: '',
weight: '',
note: ''
});8. No Optimistic UI Updates
After every create, update, or delete, the screen refetches the entire list instead of updating local state first. This makes the UI feel slower.
Expected Improvement:
Update UI optimistically, then sync with server.