Skip to content

Commit 8119f8b

Browse files
jurrajurra
committed
With this commit we clean up the usage of ElementRegistry and ElementFactory, and clearly separate their concerns.
Refactor Element Registry, Improve Command Execution, and Enhance GUI Updates Key Changes: Refactored ElementRegistry into a Singleton (Registry Role) Centralizes factory function registrations. Ensures elements are only registered once in settings.js. Provides a retrieval interface for element types. Updated CommandFactory to Dynamically Create Commands (Factory Role) Registers commands dynamically instead of storing instances. Commands now instantiate only when needed, reducing memory footprint. Simplifies command retrieval by dynamically passing parameters. Refactored CircuitService for a Hybrid Update Approach Commands now directly modify circuit state. Events are emitted only for UI updates, preventing unnecessary propagation. Removed redundant console logs. Updated GUIAdapter for Better Command Binding Uses CommandFactory to dynamically register UI commands. Ensures UI buttons trigger correct command execution. Improved AddElementCommand Execution Standardizes position initialization. Ensures UI updates are triggered correctly. Expanded and Fixed Tests Ensures ElementRegistry is initialized before tests. Improved event-driven testing in CircuitService.
1 parent f4bf6a3 commit 8119f8b

File tree

10 files changed

+123
-78
lines changed

10 files changed

+123
-78
lines changed

‎src/application/CircuitService.js‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { EventEmitter } from 'events';
22
import { Circuit } from '../domain/aggregates/Circuit.js';
33
import { Element } from '../domain/entities/Element.js';
44
import { generateId } from '../utils/idGenerator.js';
5-
import { ElementRegistry } from "../domain/factories/ElementRegistry.js";
5+
import { ElementRegistry } from "../config/settings.js";
66
import { Position } from '../domain/valueObjects/Position.js';
77
import { Properties } from '../domain/valueObjects/Properties.js';
88

@@ -31,42 +31,38 @@ export class CircuitService extends EventEmitter {
3131
this.circuit = circuit;
3232
this.elementRegistry = elementRegistry;
3333
this.on("commandExecuted", (event) => {
34-
console.log("📥 CircuitService received commandExecuted event:", event);
35-
3634
if (event.type === "addElement") {
37-
console.log("📡 CircuitService processing addElement...");
38-
3935
try {
4036
const elementFactory = this.elementRegistry.get(event.elementType);
4137
if (!elementFactory) {
42-
throw new Error(`❌ No factory registered for element type: ${event.elementType}`);
38+
throw new Error(` No factory registered for element type: ${event.elementType}`);
4339
}
4440

4541
// Ensure event.nodes is an array of Position instances
4642
if (!Array.isArray(event.nodes)) {
47-
throw new Error("❌ Nodes must be provided as an array.");
43+
throw new Error(" Nodes must be provided as an array.");
4844
}
4945

5046
// We translate node payloads from the event into Position instances
51-
const nodes = event.nodes.map(node => new Position(node.x, node.y)); // ✅ Convert to Position instances
47+
const nodes = event.nodes.map(node => new Position(node.x, node.y)); // Convert to Position instances
5248

5349
// We translate properties payloads into instances of Propertiies
54-
const properties = event.properties ? new Properties(event.properties) : new Properties(); // ✅ Convert to Properties instance
50+
const properties = event.properties ? new Properties(event.properties) : new Properties(); // Convert to Properties instance
5551

56-
// ✅ Correctly call the factory function
52+
// Correctly call the factory function
5753
const newElement = elementFactory(
5854
undefined, // Auto-generate ID
59-
nodes, // ✅ Correct nodes
60-
null, // ✅ Label (default to null)
61-
properties // ✅ Properties (default to empty object)
55+
nodes, // Correct nodes
56+
null, // Label (default to null)
57+
properties // Properties (default to empty object)
6258
);
6359

6460
console.log("✅ Created New Element:", newElement);
6561

6662
this.addElement(newElement);
67-
console.log("✅ Created New Element:", newElement);
63+
console.log("Created New Element:", newElement);
6864
} catch (error) {
69-
console.error(`❌ Error creating element: ${error.message}`);
65+
console.error(`Error creating element: ${error.message}`);
7066
}
7167
}
7268
});

‎src/config/settings.js‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,19 @@ import { WireRenderer } from '../gui/renderers/WireRenderer.js';
77
import { generateId } from '../utils/idGenerator.js';
88
import { Properties } from '../domain/valueObjects/Properties.js';
99

10-
// Configure ElementRegistry
11-
ElementRegistry.register('Resistor', (id = generateId('R'), nodes, label = null, properties = {}) =>
12-
new Resistor(id, nodes, label, new Properties(properties))
13-
);
14-
ElementRegistry.register('Wire', (id = generateId('W'), nodes, label = null, properties = {}) =>
15-
new Wire(id, nodes, label, new Properties(properties))
16-
);
10+
// Ensure elements are registered once
11+
if (ElementRegistry.getTypes().length === 0) {
12+
console.log(" Registering elements in ElementRegistry...");
13+
14+
ElementRegistry.register('Resistor', (id = generateId('R'), nodes, label = null, properties = {}) =>
15+
new Resistor(id, nodes, label, new Properties(properties))
16+
);
17+
ElementRegistry.register('Wire', (id = generateId('W'), nodes, label = null, properties = {}) =>
18+
new Wire(id, nodes, label, new Properties(properties))
19+
);
20+
}
21+
22+
console.log(" ElementRegistry after registration:", ElementRegistry.getTypes());
1723

1824
// Configure RendererFactory
1925
const rendererFactory = new RendererFactory();

‎src/domain/factories/ElementRegistry.js‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@
22
* A simple map storing factory functions for elements. Used for lookup
33
* when creating new elements based on their type.
44
*/
5-
export const ElementRegistry = {
6-
_registry: {},
5+
class ElementRegistryClass {
6+
constructor() {
7+
if (!ElementRegistryClass.instance) {
8+
this._registry = {};
9+
ElementRegistryClass.instance = this;
10+
}
11+
return ElementRegistryClass.instance;
12+
}
713

814
/**
915
* Registers a new element type.
@@ -15,7 +21,7 @@ export const ElementRegistry = {
1521
throw new Error(`Element type "${type}" is already registered.`);
1622
}
1723
this._registry[type] = factoryFunction;
18-
},
24+
}
1925

2026
/**
2127
* Retrieves the factory function for a given element type.
@@ -24,7 +30,7 @@ export const ElementRegistry = {
2430
*/
2531
get(type) {
2632
return this._registry[type];
27-
},
33+
}
2834

2935
/**
3036
* Retrieves all registered element types.
@@ -35,5 +41,11 @@ export const ElementRegistry = {
3541
}
3642
};
3743

44+
const ElementRegistry = new ElementRegistryClass();
45+
Object.freeze(ElementRegistry); // Ensures immutability
46+
47+
export { ElementRegistry };
48+
export default ElementRegistry;
49+
3850

3951

‎src/gui/adapters/GUIAdapter.js‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,12 @@ export class GUIAdapter {
4545
* @param {Object} elementRegistry - The registry of circuit elements.
4646
* @param {RendererFactory} rendererFactory - The factory for creating element renderers.
4747
*/
48-
constructor(canvas, circuitService, elementRegistry, rendererFactory) {
48+
constructor(canvas, circuitService, elementRegistry, rendererFactory, commandFactory) {
4949
this.canvas = canvas;
5050
this.circuitService = circuitService;
5151
this.elementRegistry = elementRegistry;
5252
this.circuitRenderer = new CircuitRenderer(canvas, circuitService, rendererFactory);
53+
this.commandFactory = commandFactory;
5354

5455
this.commandFactory = new CommandFactory(circuitService, this.circuitRenderer, elementRegistry);
5556
this.commandHistory = new CommandHistory();
@@ -60,19 +61,13 @@ export class GUIAdapter {
6061
* Instead of directly modifying circuit state, this now delegates execution to CommandFactory.
6162
*/
6263
bindUIControls() {
63-
this.elementRegistry.getTypes().forEach((type) => {
64-
const buttonId = `add${type}`;
65-
const button = document.getElementById(buttonId);
66-
67-
if (!button) {
68-
console.warn(`Button with ID "${buttonId}" not found.`);
69-
return;
64+
this.commandFactory.commands.forEach((createCommand, name) => {
65+
const button = document.getElementById(`button-${name}`);
66+
if (button) {
67+
button.addEventListener("click", () => createCommand().execute());
7068
}
71-
72-
button.addEventListener("click", () => this.executeCommand("addElement", type));
7369
});
74-
}
75-
70+
}
7671
/**
7772
* Executes a command by retrieving it from the CommandFactory and executing via CommandHistory.
7873
* @param {string} commandName - The name of the command to execute.
@@ -96,6 +91,9 @@ export class GUIAdapter {
9691
this.circuitRenderer.render();
9792
this.bindUIControls();
9893
this.setupCanvasInteractions();
94+
95+
// Listen for UI updates from CircuitService
96+
this.circuitService.on("update", () => this.circuitRenderer.render());
9997
}
10098

10199
/**

‎src/gui/commands/AddElementCommand.js‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { GUICommand } from "./GUICommand.js";
22

3+
/**
4+
* Command to add an element to the circuit.
5+
* Directly modifies the circuit and notifies UI updates.
6+
*/
37
export class AddElementCommand extends GUICommand {
48
constructor(circuitService, circuitRenderer, elementRegistry, elementType) {
59
super();
@@ -12,20 +16,28 @@ export class AddElementCommand extends GUICommand {
1216
execute() {
1317
console.log(`Executing AddElementCommand for ${this.elementType}`);
1418

19+
// Retrieve the factory function for the element
1520
const factory = this.elementRegistry.get(this.elementType);
1621
if (!factory) {
1722
console.error(`Factory function for element type "${this.elementType}" not found.`);
1823
return;
1924
}
2025

26+
// Generate valid node positions
2127
const nodes = [
2228
{ x: Math.random() * 800, y: Math.random() * 600 },
23-
{ x: Math.random() * 800, y: Math.random() * 600 },
24-
];
25-
const properties = {};
26-
const element = factory(undefined, nodes, null, properties);
29+
{ x: Math.random() * 800, y: Math.random() * 600 }
30+
].map(pos => new Position(pos.x, pos.y));
31+
32+
const element = factory(undefined, nodes, null, {}); // ✅ Create the element
2733

34+
// Directly modify circuit state
2835
this.circuitService.addElement(element);
36+
37+
// Notify the UI about the update
38+
this.circuitService.emit("update", { type: "addElement", element });
39+
40+
// Immediately update the renderer
2941
this.circuitRenderer.render();
3042
}
3143

‎src/gui/commands/CommandFactory.js‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,22 @@
1-
import { AddElementCommand } from "./AddElementCommand.js";
2-
import { MoveElementCommand } from "./MoveElementCommand.js";
3-
4-
/**
5-
* CommandFactory is responsible for registering and retrieving commands dynamically.
6-
*/
71
export class CommandFactory {
82
constructor() {
93
this.commands = new Map();
10-
11-
// Register available commands
12-
this.register("addElement", new AddElementCommand());
13-
this.register("moveElement", new MoveElementCommand());
144
}
155

166
/**
17-
* Registers a command instance with a unique name.
18-
* @param {string} name - The command name (e.g., "addElement").
19-
* @param {Command} commandInstance - The command instance.
7+
* Registers a new command dynamically.
208
*/
21-
register(name, commandInstance) {
22-
this.commands.set(name, commandInstance);
9+
register(name, CommandClass) {
10+
this.commands.set(name, (...args) => new CommandClass(...args));
2311
}
2412

2513
/**
26-
* Retrieves a command instance by name.
27-
* @param {string} name - The name of the command.
28-
* @returns {Command} The command instance.
14+
* Retrieves a command instance dynamically.
2915
*/
30-
get(name) {
31-
return this.commands.get(name);
16+
get(name, ...args) {
17+
if (!this.commands.has(name)) {
18+
throw new Error(`Command "${name}" not found.`);
19+
}
20+
return this.commands.get(name)(...args);
3221
}
3322
}

‎tests/domain/CircuitService.test.js‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
11
import { expect } from 'chai';
2+
import sinon from 'sinon';
23
import { Circuit } from '../../src/domain/aggregates/Circuit.js';
34
import { CircuitService } from '../../src/application/CircuitService.js';
45
import { MockElement } from './MockElement.js'; // A mock element class for testing
56
import { Position } from '../../src/domain/valueObjects/Position.js';
7+
import { ElementRegistry } from '../../src/config/settings.js';
68

79
describe('CircuitService Tests', () => {
810
let circuit;
911
let circuitService;
12+
let emitSpy;
1013

1114
beforeEach(() => {
1215
circuit = new Circuit();
13-
circuitService = new CircuitService(circuit);
16+
17+
circuitService = new CircuitService(circuit, ElementRegistry);
18+
19+
// Ensure emitSpy is correctly tracking circuitService.emit
20+
emitSpy = sinon.spy(circuitService, "emit");
21+
1422
});
1523

1624
describe('addElement', () => {
@@ -41,8 +49,35 @@ describe('CircuitService Tests', () => {
4149
expect(() => circuitService.addElement(element2)).to.throw(
4250
'Element with id E1 is already in the circuit.'
4351
);
52+
// delete Element
53+
circuitService.deleteElement('E1');
54+
4455
});
45-
});
56+
57+
it("should add an element to the circuit and emit an update", function (done) {
58+
this.timeout(5000); // ✅ Only works in function declarations
59+
60+
const element = new MockElement("E1", [
61+
new Position(10, 20),
62+
new Position(30, 40),
63+
]);
64+
65+
circuitService.addElement(element);
66+
67+
process.nextTick(() => {
68+
try {
69+
expect(circuit.elements).to.include(element);
70+
71+
const emittedElement = emitSpy.getCalls()[0].args[1].element;
72+
expect(emittedElement.id).to.equal(element.id);
73+
74+
done(); // Ensures Mocha completes successfully
75+
} catch (err) {
76+
done(err); // Ensures Mocha captures errors
77+
}
78+
});
79+
});
80+
});
4681

4782
describe('deleteElement', () => {
4883
it('should delete an element from the circuit', () => {

‎tests/domain/ElementFactory.test.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,18 @@ import { MockElement } from "./MockElement.js";
55
import { Position } from "../../src/domain/valueObjects/Position.js";
66

77
describe("ElementFactory Tests", () => {
8+
ElementRegistry.register("MockElement", (id, nodes, properties) =>
9+
new MockElement(id, nodes, properties)
10+
);
11+
812
beforeEach(() => {
9-
// Register a mock element type
10-
ElementRegistry.register("MockElement", (id, nodes, properties) =>
11-
new MockElement(id, nodes, properties)
12-
);
13+
// Nothing to do before each test for now
14+
1315
});
1416

1517
afterEach(() => {
1618
// Reset registry after each test
17-
ElementRegistry._registry = {};
19+
// ElementRegistry._registry = {};
1820
});
1921

2022
it("should create an instance of a registered element", () => {

‎tests/gui/GUIAdapter.test.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import '../../src/config/settings.js'; // ✅ Ensures registration happens before tests
12
import { expect } from 'chai';
23
import sinon from 'sinon';
34
import { setupJsdom } from '../setup/jsdomSetup.js';
@@ -12,7 +13,10 @@ describe('GUIAdapter Tests', () => {
1213
let canvas;
1314
let guiAdapter;
1415

16+
console.log("🔍 ElementRegistry before test:", ElementRegistry.getTypes());
17+
1518
beforeEach(() => {
19+
console.log("🔍 ElementRegistry before test:", ElementRegistry.getTypes());
1620
setupJsdom();
1721

1822
// Add required buttons to the DOM
@@ -41,9 +45,6 @@ describe('GUIAdapter Tests', () => {
4145
removeEventListener: sinon.spy(),
4246
};
4347

44-
console.log("🔎 Available Element Types:", ElementRegistry.getTypes()); // ✅ Debugging output
45-
// console.log("ElementRegistry:", ElementRegistry); // ✅ Debugging output
46-
// Create GUIAdapter instance
4748
const circuit = new Circuit();
4849
const circuitService = new CircuitService(circuit, ElementRegistry);
4950
guiAdapter = new GUIAdapter(canvas, circuitService, ElementRegistry, rendererFactory);

‎tests/logic/CircuitService.test.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,6 @@ describe("CircuitService Tests", () => {
4040
});
4141

4242
it("should process a commandExecuted event for addElement", () => {
43-
console.log("🔎 Available Element Types:", ElementRegistry.getTypes());
44-
45-
// Register MockElement
46-
ElementRegistry.register("MockElement", (id, nodes, label, properties) =>
47-
new MockElement(id, nodes, label, properties)
48-
);
4943

5044
// Simulating raw position data (as sent by events)
5145
const nodes = [

0 commit comments

Comments
 (0)