Skip to content

Target refactor: simplify codebase structures and optimize risk parsing performance#392

Open
lalit-t0251068 wants to merge 6 commits into
ThalesGroup:masterfrom
abhi3121:Target-refactor
Open

Target refactor: simplify codebase structures and optimize risk parsing performance#392
lalit-t0251068 wants to merge 6 commits into
ThalesGroup:masterfrom
abhi3121:Target-refactor

Conversation

@lalit-t0251068

Copy link
Copy Markdown

refactor: simplify codebase structures and optimize risk parsing performance

📝 Overview

This pull request introduces a comprehensive series of codebase simplifications and performance optimizations across 6 core files within the lib module. These improvements focus on removing redundant wrappers, simplifying complex syntactic patterns (such as immediately-invoked anonymous functions and nested ternaries), and resolving a high-impact $O(N \times M)$ parsing loop bottleneck.

All changes strictly preserve existing functional behavior and public API contracts, verified by 100% success of the local test suite.


🚀 Key Improvements & Performance Impact

1. Risk Parsing Optimization ($O(N \times M) \rightarrow O(1)$ lookup)

  • File: lib/src/api/xml-json/alter-isra/alter-risks.js
  • Issue: Inside the per-risk .forEach loop, the lookup maps businessAssets and supportingAssets were completely rebuilt from scratch on every single iteration from static input arrays. This was a classic performance hotspot for large SRA/XML datasets.
  • Refactor: Moved the map constructions outside of the loop. They are now built exactly once and can be retrieved in $O(1)$ constant time inside the loop, significantly reducing CPU cycles and garbage collection overhead.

2. Spelling Corrections Streamlining (Chained .replace)

  • File: lib/src/api/xml-json/parser.js
  • Issue: The editName helper function relied on five separate, chained .match() checks paired with ternary expressions to sanitize XML tags.
  • Refactor: Chained .replace operations directly. Since String.prototype.replace natively returns the original string unchanged if there is no match, this removes the redundant conditional checks and matches, improving both speed and readability.

3. Redundant Closure & IIFE Removals

  • Files: lib/src/utility-global.js & lib/src/api/xml-json/alter-isra/alter-isra.js
  • Refactor:
    • Simplified utility-global.js from a nested Immediately Invoked Function Expression (IIFE) with warning suppressions (no-unused-vars) into a standard CommonJS module directly exporting a clean counter closure.
    • Removed a redundant self-invoking function wrapper (israJSONDataCopy, xmlDataCopy) => { ... })(israJSONData, xmlData) inside alterISRA and executed the logic directly in the parent scope.

4. Code Clarity & Syntax Flattening

  • Files: lib/src/api/xml-json/alter-isra/alter-business-assets.js & lib/src/api/xml-json/populate-class.js
  • Refactor:
    • Replaced a highly convoluted immediately-invoked destructuring function (({ props }) => ({ props }))(ba) with standard ES6 destructuring and object assignment for ba.businessAssetProperties.
    • Flattened redundant ternary checks like highestBAId? highestBAId: businessAssetId where the variable is guaranteed to be a truthy pre-incremented number.

🛠️ Verification & Test Coverage

All modifications have been validated locally using Jest:

  • Test Suites: 19 passed, 19 total
  • Tests: 311 passed, 311 total
  • Behavior: 0 regressions introduced. External API inputs, schema validations, and file serialization formats remain 100% compatible.

📁 Files Modified

File Path Description of Refactor
lib/src/utility-global.js Removed IIFE, clean CommonJS counter closure export.
lib/src/api/xml-json/parser.js Direct chaining of .replace in editName.
lib/src/api/xml-json/alter-isra/alter-business-assets.js Clean ES6 object destructuring in businessAssetProperties.
lib/src/api/xml-json/alter-isra/alter-isra.js Redundant parameter-repassing IIFE wrapper removed.
lib/src/api/xml-json/alter-isra/alter-risks.js Optimized lookup maps extraction out of loop.
lib/src/api/xml-json/populate-class.js Removed redundant ternary assertions.
SIMPLIFICATION_PLAN.md Detailed analysis document compiled for review.

SINGH Abhishek Kumar and others added 6 commits June 24, 2026 11:34
Introduces Claude Code configuration for AI-assisted development on ISRA.

CLAUDE.md
- Documents monorepo structure (lib/ vs app/), IPC architecture, ISO 27005
  data model, validation pattern, file formats, testing, linting, and
  Electron security constraints — giving Claude full project context.

.claude/settings.json (hooks)
- PostToolUse: auto-runs ESLint --fix on any lib/**/*.js file after an edit
- PreToolUse: blocks direct edits to package-lock.json

.claude/agents/security-reviewer.md
- Electron security specialist agent: reviews against Electron hardening
  rules, OWASP Desktop App Top 10, IPC input sanitization, XXE/XSS risks
  in xml-json/, and file path traversal in data-load/data-store/

.claude/agents/test-writer.md
- Jest test generator: writes unit and integration tests for lib/src/
  following existing patterns (AJV validation, round-trip load/save,
  handler mocking)

.claude/skills/project-conventions/SKILL.md
- Loads ISRA-specific conventions (data model hierarchy, IPC pattern,
  validation approach, URL scheme allowlist) before code generation

.claude/skills/run-tests/SKILL.md
- One-command skill that runs the full Jest suite with coverage for lib/

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Excludes files that waste context without helping Claude understand the code:
- node_modules/, dist/, coverage/ — generated/installed, not source
- lib/doc/APIdocumentation/ — generated JSDoc output
- app/src/asset/ images — binary files Claude cannot use
- lib/test/integration/fixtures/ — large JSON/XML test data (read on demand)
- doc/*.xlsx — binary Office document
- package-lock.json files — auto-generated, never hand-edited

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ormance

- Simplify utility counter closure by removing redundant IIFE wrapper and unused parameters.
- Optimize alterRisks parsing loop by moving businessAssets and supportingAssets lookup map constructions outside the per-risk forEach loop, reducing overhead from O(N*M) to O(1) inside loop.
- Streamline parser.js editName spelling corrections by replacing nested .match checks with chained .replace operations.
- Remove redundant immediately invoked anonymous destructuring functions in alter-business-assets.js.
- Remove redundant self-invoking function wrappers in alter-isra.js.
- Clean up guaranteed-truthy ternary checks in populate-class.js.
- Save full analysis in SIMPLIFICATION_PLAN.md.

All 311 unit and integration tests passing successfully.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants