[lab 5 test] Eight store helpers, stock reviewer

[sergical]

Test pull request for lab 5. Do not merge. The stock reviewer runs on eight small files.

Created with Claude Code

[github-actions]

Verdict: Request changes — multiple correctness bugs, at least one crash-causing, must be fixed before merge.

Correctness (blocking)

1. cart-total.ts — off-by-one, will crash. Loop uses i <= items.length, so on the last iteration items[i] is undefined and .price/.qty throw a TypeError. Every non-trivial call to cartTotal fails. Fix: i < items.length.

2. refund-window.ts — wrong unit divisor. getTime() returns milliseconds, but the code divides by 86400 (seconds/day) instead of 86400000 (ms/day). This makes days ~1000x too large, so canRefund will return false almost immediately instead of allowing a real 30-day window. This is a significant behavioral bug.

3. currency.ts — no decimal formatting. "$" + cents / 100 produces "$1" for 100 cents (should be "$1.00") and "$10.5" for 1050 cents (should be "$10.50"). Needs .toFixed(2).

4. loyalty.ts — rounds too early, reintroduces fractions. Math.round(totalCents / 100) * 1.5 can still yield non-integer points (e.g. 100 cents → 1.5 points). If points must be integers, round the final product instead: Math.round((totalCents / 100) * 1.5).

5. order-id.ts — fragile parsing. parseInt(last) (no radix) silently drops prefixes/leading zeros (e.g. "ORD-0099" → NaN, "00099" → "100" losing zero-padding), producing malformed or "NaN" IDs with no error handling. Needs validation and a radix argument at minimum.

6. discount.ts — ambiguous contract, easy misuse. Nothing indicates whether percent is a fraction (0.2) or whole number (20); passing 20 silently produces a large negative total. No clamping/validation of range.

7. email.ts — validation far too weak. value.includes("@") accepts "@", "@@@", or strings with embedded spaces as "valid" emails. If this is meant for real validation (not a placeholder), it needs a proper check.

8. shipping.ts — no input validation. Negative/zero/NaN weightKg silently returns a normal rate instead of erroring; worth confirming the >5 boundary matches intended business rules.

Style / maintainability (non-blocking)

• Magic numbers throughout (100 in currency.ts/loyalty.ts, 86400 in refund-window.ts, 5/12/6/2 in shipping.ts, 1.5 in loyalty.ts) should be named constants (e.g. CENTS_PER_DOLLAR, MS_PER_DAY, POINTS_PER_DOLLAR, HEAVY_THRESHOLD_KG) for readability and single-source-of-truth maintenance.
• discount.ts: rename percent to rate (or document that it's a 0–1 fraction) to avoid the ambiguity noted above.
• order-id.ts: parseInt should always take an explicit radix (parseInt(last, 10)), independent of the correctness issue.
• None of the new utility functions have JSDoc/comments documenting expected input ranges/formats (e.g., is weightKg always positive? is last guaranteed numeric?), nor are there accompanying tests — worth adding given these look like shared business-logic utilities.

Summary

The diff adds several small utility functions, but at least two are outright broken (cartTotal crashes on any input, canRefund's day math is off by a factor of ~1000), and several others have latent correctness risks (formatting, rounding, parsing, weak validation) that should be addressed before merging. Style issues (magic numbers, missing docs) are secondary but worth cleaning up in the same pass.

[github-actions]

Verdict

Request changes — several new utility functions contain real logic bugs (off-by-one, unit mismatch, format bugs) that will cause runtime errors or incorrect behavior; style/clarity issues are secondary but numerous.

Correctness (blocking)

1. cart-total.ts — for (let i = 0; i <= items.length; i++) is an off-by-one bug: on the last iteration items[items.length] is undefined, so .price/.qty access throws a TypeError. Should be i < items.length (or better, use for...of/reduce).

2. refund-window.ts — getTime() returns milliseconds, but the code divides by 86400 (seconds-per-day) instead of 86400000. This makes days ~1000x too large, so canRefund will almost always return false, breaking the 30‑day refund window entirely. Also doesn't guard against now < deliveredAt (negative days would incorrectly pass).

3. discount.ts — applyDiscount treats percent as a 0–1 fraction, but the parameter name strongly implies a 0–100 percentage. A caller passing 20 for "20%" gets a wildly negative total. Needs either a rename to fraction/rate, an explicit /100 conversion, or input validation/clamping.

4. loyalty.ts — Math.round(totalCents / 100) rounds before applying the 1.5 multiplier, skewing results, and the final value isn't rounded to a whole number even though "points" are normally integers (e.g. pointsFor(100) === 1.5).

5. order-id.ts — parseInt(last) has no radix and will mis-parse hex-looking strings; more importantly, leading zero-padding in last (e.g. "00099") is lost after +1 ("100" instead of "00100"), which is a likely regression if IDs are zero-padded. NaN inputs also silently produce "NaN" with no error handling.

6. email.ts — isEmail only checks for "@", accepting invalid values like "@", "a@", "@b". Fine as a placeholder, but misleading given the name — should not be relied on for real validation.

7. currency.ts — "$" + cents/100 doesn't fix decimal places, so formatUsd(150) → "$1.5" rather than "$1.50". Use .toFixed(2).

8. shipping.ts — no guard against negative/zero weightKg, which would still return a positive cost; confirm the weightKg === 5 boundary matches the intended pricing tier.

Style / Clarity (non-blocking)

• Magic numbers throughout (100, 1.5, 86400, 30, 5, 12, 6, 2) should be named constants (CENTS_PER_DOLLAR, MS_PER_DAY, REFUND_WINDOW_DAYS, WEIGHT_THRESHOLD_KG, etc.) to make units and business rules self-documenting.
• cart-total.ts: prefer idiomatic iteration (for...of or .reduce) over manual indexing.
• order-id.ts: use parseInt(last, 10) explicitly; consider renaming last to lastOrderId.
• discount.ts: rename percent to fraction/rate to match actual semantics.
• email.ts: rename isEmail to something like hasAtSign/looksLikeEmail, or document the limited scope.
• loyalty.ts: pointsFor is vague; consider loyaltyPointsForTotal.
• None of the new files have JSDoc explaining units (cents vs. dollars, ms vs. days) or parameter expectations — worth adding given how easily these are confused (as evidenced by the bugs above).

Summary

The PR introduces eight small utilities, but at least five (cart-total, refund-window, discount, loyalty, order-id) have functional bugs ranging from crashes to silently wrong business results. Recommend fixing the correctness issues before merge, then addressing the naming/magic-number cleanup.