|
|
@@ -224,8 +224,52 @@ When a cell or range is selected, show a compact set of floating action buttons
|
|
|
|
|
|
---
|
|
|
|
|
|
-## 12. Code Refactoring
|
|
|
+## 12. Code Health & Refactoring
|
|
|
|
|
|
-- [ ] **12.1 Rewrite `create-all-references` to use `walk-modify-data`**
|
|
|
+> The items below were identified from a partial read of the codebase. The remaining files should be audited for similar issues before this section is considered complete.
|
|
|
+
|
|
|
+### Bugs & Correctness
|
|
|
+
|
|
|
+- [ ] **12.1 Fix duplicate `:inbound` entries**
|
|
|
+
|
|
|
+ `notify-references` uses `conj` on a list with no deduplication. A formula like `=A1 + A1` adds A1's back-reference twice. `denotify-references` removes only the first match via `filter`, leaving one stale copy. Use a set for `:inbound`, or deduplicate on insert.
|
|
|
+
|
|
|
+- [ ] **12.2 Replace lazy seq in `:inbound` with a concrete collection**
|
|
|
+
|
|
|
+ `denotify-references` stores the result of `(partial filter ...)` — a lazy sequence — into the app db. Lazy seqs in re-frame state can cause subtle equality and inspection issues. Replace with `(into [] (filter ...))`.
|
|
|
+
|
|
|
+- [ ] **12.3 Add a termination guard to `evaluate-all`**
|
|
|
+
|
|
|
+ The retry loop — `(recur data (concat (rest queue) (list cur)))` — re-queues cells with unsatisfied dependencies indefinitely. Cycle detection in `evaluate-from-cell` should prevent an infinite loop in practice, but there is no explicit guard. Add a visited set or iteration counter as a safety net.
|
|
|
+
|
|
|
+### Deprecations
|
|
|
+
|
|
|
+- [ ] **12.4 Replace deprecated `on-keyPress` with `on-key-down`**
|
|
|
+
|
|
|
+ The cell's enter-key handler in `views/sheet.cljs` uses `:on-keyPress`, which is deprecated in both React and the underlying browser API. Move to `:on-key-down`.
|
|
|
+
|
|
|
+### Performance
|
|
|
+
|
|
|
+- [ ] **12.5 Address unbounded `evaluate-expression` memoization cache**
|
|
|
+
|
|
|
+ `evaluate-expression` is memoized on `(expression, variables-map)`. Every unique combination ever computed is cached for the lifetime of the page. In a long editing session this becomes a gradual memory leak. Consider a bounded LRU cache or explicit cache invalidation.
|
|
|
+
|
|
|
+### Cleanup
|
|
|
+
|
|
|
+- [ ] **12.6 Remove `println` debug statements**
|
|
|
+
|
|
|
+ Every event handler and subscription (`::table-data` in particular) fires `println` on every call. This floods the console and makes debugging new code harder. Remove or gate behind a dev-mode flag.
|
|
|
+
|
|
|
+- [ ] **12.7 Consolidate hardcoded table dimensions**
|
|
|
+
|
|
|
+ `maxrow = 20` and `maxcol = "G"` are hardcoded in `views/sheet.cljs` while `min-max-row` and `min-max-col` defined in `db.cljs` go unused. The view should read from the db values, leaving a single source of truth. (Related: 1.3.)
|
|
|
+
|
|
|
+### Refactoring
|
|
|
+
|
|
|
+- [ ] **12.8 Break up `gather-variables-and-evaluate-cell`**
|
|
|
+
|
|
|
+ At ~35 lines with six boolean flags and a multi-branch `cond`, the function is near its readability limit. Adding another error type or evaluation mode will tip it over. Consider extracting the disqualification checks and the variable-collection step into named helpers.
|
|
|
+
|
|
|
+- [ ] **12.9 Rewrite `create-all-references` to use `walk-modify-data`**
|
|
|
|
|
|
Both functions perform a `reduce-kv` double-walk over the data map. Eliminate the duplication by having `create-all-references` call `walk-modify-data`.
|