Refactor: consolidate duplicate code, fix coverage, update docs (#323)

* Refactor haro.js: consolidate duplicate code patterns

Extract #freezeResult(), #fromCache(), and #toCache() private helpers to eliminate
repeated Object.freeze return patterns across 10+ methods. Consolidate #getIndexKeys
and #getIndexKeysForWhere into a single parametrized #getIndexKeysFrom method
(2 methods → 1, removing 62 lines of duplicate code).

* test: cover #setIndex() branch recreated after store clear

Add 'should recreate index after clear on same store' test to achieve
100% line coverage across haro.js.

* docs: expand AGENTS.md with internal helpers, patterns, and test strategy

* docs: update API and technical docs with internal helper methods

- Remove clone(), freeze(), merge() from public API (all are private)
- Add Internal Helper Methods section documenting #freezeResult, #fromCache,
  #toCache, #getIndexKeysFrom
- Update Immutability Model to reference centralized #freezeResult()
- Update cache mutation protection to reference #fromCache/#toCache
- Clarify internal vs public utility methods

* docs: remove clone()/merge() from README Quick Overview (internal only)

Author: avoidwork

AGENTS.md

@@ -49,6 +49,49 @@ npm run benchmark         # Run benchmarks
 - Adheres to DRY, YAGNI, and SOLID principles
 - Follows OWASP security guidance
 
+## Internal Helper Methods
+- `#freezeResult(result)` - Freezes individual values or arrays when immutable mode is enabled
+- `#fromCache(cached)` - Returns cloned result (non-immutable) or frozen result (immutable) from cache
+- `#toCache(cacheKey, records)` - Stores results in cache if enabled
+- `#getIndexKeysFrom(arg, source, getValueFn)` - Generates composite index keys using a getter callback
+- `#getNestedValue(obj, path)` - Retrieves nested values using dot notation (e.g., `user.profile.city`)
+- `#sortKeys(a, b)` - Type-aware comparator: strings use `localeCompare`, numbers use subtraction, mixed types coerced to string
+- `#merge(a, b, override)` - Deep merges values, skips prototype pollution keys (`__proto__`, `constructor`, `prototype`)
+- `#invalidateCache()` - Clears cache if enabled and not in batch mode
+- `#getCacheKey(domain, ...args)` - Generates SHA-256 hash cache key from arguments
+- `#clone(arg)` - Deep clones values via `structuredClone` or JSON fallback
+
+## Immutable Mode
+- Use `#freezeResult()` to return frozen data instead of inline `Object.freeze()` checks
+- Applies to: `get()`, `find()`, `filter()`, `limit()`, `map()`, `toArray()`, `sort()`, `sortBy()`, `search()`, `where()`
+- Cached results are also frozen/cloned via `#fromCache()` when read
+- `#merge()` preserves version snapshots by freezing cloned originals before versioning
+
+## Caching
+- Cache is opt-in via `cache: true` in constructor
+- Automatically invalidates on all write operations (set, delete, clear, reindex, setMany, deleteMany, override)
+- Does NOT invalidate during batch operations (`#inBatch = true`), only after batch completes
+- `search()` and `where()` use multi-domain cache keys: `search_HASH` / `where_HASH`
+- LRU cache size defaults to 1000 (`CACHE_SIZE_DEFAULT`)
+
+## Indexing
+- `#index` config array persists across `clear()` - `clearIndexes` is separate
+- After `clear()`, `#indexes` Map is emptied but `#index` config remains
+- Setting new records after `clear()` triggers lazy re-creation of index Maps in `#setIndex()`
+- Composite indexes use delimiter (default `|`) to join sorted field names
+- Use `#getIndexKeysFrom()` with appropriate getter callbacks for data objects vs where clauses
+
+## Batch Operations
+- `setMany()` and `deleteMany()` set `#inBatch = true` to skip indexing during individual operations
+- Indexing is deferred to `reindex()` call after batch completes
+- Nested batch calls (calling setMany within setMany) throw errors
+- `#inBatch` affects versioning too - versions are not saved during batch operations
+
+## Test Strategy Tips
+- Hard-to-reach branches often involve state transitions (e.g., `clear()` → `set()` on same store)
+- Coverage gaps frequently involve conditional logic on `#inBatch`, `#immutable`, or `#cacheEnabled`
+- Add tests that explicitly combine features (immutable + indexing + batch + caching)
+
 ## Important Notes
 - The `immutable` option freezes data for immutability
 - Indexes improve query performance for `find()` and `where()` operations
@@ -62,3 +105,10 @@ npm run benchmark         # Run benchmarks
 - Cache invalidates on all write operations but preserves statistics
 - `search()` and `where()` are async methods - use `await` when calling
 - Cache statistics persist for the lifetime of the Haro instance
+
+## Refactoring Patterns
+- Centralize immutable freezing in `#freezeResult()` - never use inline `Object.freeze()` for return values
+- Centralize cache read patterns in `#fromCache(cached)` and cache write in `#toCache(key, records)`
+- Consolidate index key generation in `#getIndexKeysFrom()` using a getter callback parameter
+- When DRY requires a single function to handle both data objects and where clauses, use a `getValueFn` callback parameter
+- Always run `npm test` after structural refactors - helper extraction changes call signatures

README.md

@@ -425,7 +425,7 @@ For complete API documentation with all methods and examples, see [API.md](https
 - **Core Methods**: `set()`, `get()`, `delete()`, `has()`, `clear()`
 - **Query Methods**: `find()`, `where()`, `search()`, `filter()`, `sortBy()`, `limit()`
 - **Batch Operations**: `setMany()`, `deleteMany()`
-- **Utility Methods**: `clone()`, `merge()`, `toArray()`, `dump()`, `override()`
+- **Utility Methods**: `toArray()`, `dump()`, `override()`
 - **Properties**: `size`, `registry`
 
 ## Troubleshooting

coverage.txt

@@ -4,8 +4,8 @@
 ℹ --------------------------------------------------------------
 ℹ src           |        |          |         | 
 ℹ  constants.js | 100.00 |   100.00 |  100.00 | 
-ℹ  haro.js      | 100.00 |    97.92 |   98.73 | 
+ℹ  haro.js      | 100.00 |    97.86 |   97.62 | 
 ℹ --------------------------------------------------------------
-ℹ all files     | 100.00 |    97.93 |   98.73 | 
+ℹ all files     | 100.00 |    97.86 |   97.62 | 
 ℹ --------------------------------------------------------------
 ℹ end of coverage report

dist/haro.cjs

@@ -256,6 +256,45 @@ class Haro {
 		/* node:coverage ignore */ return JSON.parse(JSON.stringify(arg));
 	}
 
+	/**
+	 * Freezes a result when immutable mode is enabled.
+	 * @param {Array|Object} result - Value to freeze
+	 * @returns {Array|Object} Frozen or unchanged result
+	 */
+	#freezeResult(result) {
+		if (this.#immutable) {
+			if (Array.isArray(result)) {
+				for (let i = 0, len = result.length; i < len; i++) {
+					Object.freeze(result[i]);
+				}
+				Object.freeze(result);
+			} else {
+				Object.freeze(result);
+			}
+		}
+		return result;
+	}
+
+	/**
+	 * Returns a cached result, cloned or frozen based on immutable mode.
+	 * @param {*} cached - Cached value
+	 * @returns {*} Cloned (non-immutable) or frozen (immutable) result
+	 */
+	#fromCache(cached) {
+		return this.#immutable ? Object.freeze(cached) : this.#clone(cached);
+	}
+
+	/**
+	 * Stores results in cache if enabled.
+	 * @param {*} cacheKey - Cache key
+	 * @param {Array} records - Result records to cache
+	 */
+	#toCache(cacheKey, records) {
+		if (this.#cacheEnabled) {
+			this.#cache.set(cacheKey, records);
+		}
+	}
+
 	/**
 	 * Deletes a record and removes it from all indexes.
 	 * @param {string} [key=STRING_EMPTY] - Key to delete
@@ -368,7 +407,7 @@ class Haro {
 			const idx = this.#indexes.get(i);
 			if (!idx) return;
 			const values = i.includes(this.#delimiter)
-				? this.#getIndexKeys(i, this.#delimiter, data)
+				? this.#getIndexKeysFrom(i, data, (f, s) => this.#getNestedValue(s, f))
 				: Array.isArray(this.#getNestedValue(data, i))
 					? this.#getNestedValue(data, i)
 					: [this.#getNestedValue(data, i)];
@@ -415,58 +454,20 @@ class Haro {
 	}
 
 	/**
-	 * Generates index keys for composite indexes from data object.
+	 * Generates index keys from source object using a value getter function.
 	 * @param {string} arg - Composite index field names
 	 * @param {string} delimiter - Field delimiter
-	 * @param {Object} data - Data object
+	 * @param {Object} source - Source object (data or where clause)
+	 * @param {Function} getValueFn - Function(field, source) => value
 	 * @returns {string[]} Index keys
 	 */
-	#getIndexKeys(arg, delimiter, data) {
+	#getIndexKeysFrom(arg, source, getValueFn) {
 		const fields = arg.split(this.#delimiter).sort(this.#sortKeys);
 		const result = [STRING_EMPTY];
 		const fieldsLen = fields.length;
 		for (let i = 0; i < fieldsLen; i++) {
 			const field = fields[i];
-			const fieldValue = this.#getNestedValue(data, field);
-			const values = Array.isArray(fieldValue) ? fieldValue : [fieldValue];
-			const newResult = [];
-			const resultLen = result.length;
-			const valuesLen = values.length;
-			for (let j = 0; j < resultLen; j++) {
-				const existing = result[j];
-				for (let k = 0; k < valuesLen; k++) {
-					const value = values[k];
-					const newKey = i === 0 ? value : `${existing}${this.#delimiter}${value}`;
-					newResult.push(newKey);
-				}
-			}
-			result.length = 0;
-			result.push(...newResult);
-		}
-		return result;
-	}
-
-	/**
-	 * Generates index keys for where object (handles both dot notation and direct access).
-	 * @param {string} arg - Composite index field names
-	 * @param {string} delimiter - Field delimiter
-	 * @param {Object} where - Where object
-	 * @returns {string[]} Index keys
-	 */
-	#getIndexKeysForWhere(arg, delimiter, where) {
-		const fields = arg.split(this.#delimiter).sort(this.#sortKeys);
-		const result = [STRING_EMPTY];
-		const fieldsLen = fields.length;
-		for (let i = 0; i < fieldsLen; i++) {
-			const field = fields[i];
-			// Check if field exists directly in where object first (for dot notation keys)
-			let fieldValue;
-			if (field in where) {
-				fieldValue = where[field];
-				/* node:coverage ignore next 4 */
-			} else {
-				fieldValue = this.#getNestedValue(where, field);
-			}
+			const fieldValue = getValueFn(field, source);
 			const values = Array.isArray(fieldValue) ? fieldValue : [fieldValue];
 			const newResult = [];
 			const resultLen = result.length;
@@ -512,7 +513,9 @@ class Haro {
 
 		const index = this.#indexes.get(compositeKey);
 		if (index) {
-			const keys = this.#getIndexKeysForWhere(compositeKey, this.#delimiter, where);
+			const keys = this.#getIndexKeysFrom(compositeKey, where, (f, s) =>
+				f in s ? s[f] : this.#getNestedValue(s, f),
+			);
 			const keysLen = keys.length;
 			for (let i = 0; i < keysLen; i++) {
 				const v = keys[i];
@@ -525,11 +528,7 @@ class Haro {
 			}
 		}
 
-		const records = Array.from(result, (i) => this.get(i));
-		if (this.#immutable) {
-			return Object.freeze(records);
-		}
-		return records;
+		return this.#freezeResult(Array.from(result, (i) => this.get(i)));
 	}
 
 	/**
@@ -550,10 +549,7 @@ class Haro {
 				result.push(value);
 			}
 		});
-		if (this.#immutable) {
-			return Object.freeze(result);
-		}
-		return result;
+		return this.#freezeResult(result);
 	}
 
 	/**
@@ -587,10 +583,7 @@ class Haro {
 		if (result === undefined) {
 			return null;
 		}
-		if (this.#immutable) {
-			return Object.freeze(result);
-		}
-		return result;
+		return this.#freezeResult(result);
 	}
 
 	/**
@@ -629,12 +622,7 @@ class Haro {
 		if (typeof max !== STRING_NUMBER) {
 			throw new Error(STRING_ERROR_LIMIT_MAX_TYPE);
 		}
-		let result = this.registry.slice(offset, offset + max).map((i) => this.get(i));
-		if (this.#immutable) {
-			result = Object.freeze(result);
-		}
-
-		return result;
+		return this.#freezeResult(this.registry.slice(offset, offset + max).map((i) => this.get(i)));
 	}
 
 	/**
@@ -651,11 +639,7 @@ class Haro {
 		}
 		let result = [];
 		this.forEach((value, key) => result.push(fn(value, key)));
-		if (this.#immutable) {
-			result = Object.freeze(result);
-		}
-
-		return result;
+		return this.#freezeResult(result);
 	}
 
 	/**
@@ -762,7 +746,7 @@ class Haro {
 			cacheKey = await this.#getCacheKey(STRING_CACHE_DOMAIN_SEARCH, value, index);
 			const cached = this.#cache.get(cacheKey);
 			if (cached !== undefined) {
-				return this.#immutable ? Object.freeze(cached) : this.#clone(cached);
+				return this.#fromCache(cached);
 			}
 		}
 
@@ -799,14 +783,8 @@ class Haro {
 		}
 		const records = Array.from(result, (key) => this.get(key));
 
-		if (this.#cacheEnabled) {
-			this.#cache.set(cacheKey, records);
-		}
-
-		if (this.#immutable) {
-			return Object.freeze(records);
-		}
-		return records;
+		this.#toCache(cacheKey, records);
+		return this.#freezeResult(records);
 	}
 
 	/**
@@ -876,7 +854,7 @@ class Haro {
 				this.#indexes.set(field, idx);
 			}
 			const values = field.includes(this.#delimiter)
-				? this.#getIndexKeys(field, this.#delimiter, data)
+				? this.#getIndexKeysFrom(field, data, (f, s) => this.#getNestedValue(s, f))
 				: Array.isArray(this.#getNestedValue(data, field))
 					? this.#getNestedValue(data, field)
 					: [this.#getNestedValue(data, field)];
@@ -959,10 +937,7 @@ class Haro {
 			return mapped;
 		});
 
-		if (this.#immutable) {
-			return Object.freeze(result);
-		}
-		return result;
+		return this.#freezeResult(result);
 	}
 
 	/**
@@ -974,11 +949,7 @@ class Haro {
 	toArray() {
 		const result = Array.from(this.#data.values());
 		if (this.#immutable) {
-			const resultLen = result.length;
-			for (let i = 0; i < resultLen; i++) {
-				Object.freeze(result[i]);
-			}
-			Object.freeze(result);
+			return this.#freezeResult(result);
 		}
 
 		return result;
@@ -1058,7 +1029,7 @@ class Haro {
 			cacheKey = await this.#getCacheKey(STRING_CACHE_DOMAIN_WHERE, predicate, op);
 			const cached = this.#cache.get(cacheKey);
 			if (cached !== undefined) {
-				return this.#immutable ? Object.freeze(cached) : this.#clone(cached);
+				return this.#fromCache(cached);
 			}
 		}
 
@@ -1130,14 +1101,8 @@ class Haro {
 				}
 			}
 
-			if (this.#cacheEnabled) {
-				this.#cache.set(cacheKey, results);
-			}
-
-			if (this.#immutable) {
-				return Object.freeze(results);
-			}
-			return results;
+			this.#toCache(cacheKey, results);
+			return this.#freezeResult(results);
 		}
 	}
 }