Skip to content

Commit 469f44e

Browse files
committed
refactor: reduce reliance on implicit global state in Logo subsystem
Introduce LogoDependencies for explicit dependency injection in Logo. Remove meSpeak references following its removal from master. Aligned with latest master and applied Prettier formatting.
1 parent 201c502 commit 469f44e

File tree

4 files changed

+204
-124
lines changed

4 files changed

+204
-124
lines changed

js/LogoDependencies.js

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
* @file LogoDependencies.js
1414
* @description Explicit dependency container for the Logo execution engine.
1515
*
16-
* This class makes Logo's dependencies explicit rather than accessing them
17-
* through the global Activity facade, improving testability and code clarity.
16+
* This class manages Logo's explicit dependencies.
1817
*/
1918

2019
/**
@@ -62,7 +61,7 @@ class LogoDependencies {
6261
* @param {Function} deps.callbacks.onStopTurtle - Called when turtle stops
6362
* @param {Function} deps.callbacks.onRunTurtle - Called when turtle runs
6463
*
65-
* @param {Object} [deps.meSpeak] - Optional speech synthesis object
64+
6665
*/
6766
constructor({
6867
blocks,
@@ -73,7 +72,15 @@ class LogoDependencies {
7372
storage,
7473
config,
7574
callbacks,
76-
meSpeak = null
75+
76+
instruments = null,
77+
instrumentsFilters = null,
78+
instrumentsEffects = null,
79+
widgetWindows = null,
80+
utils = null,
81+
Singer = null,
82+
Tone = null,
83+
classes = null
7784
}) {
7885
// Validate required dependencies
7986
if (!blocks) {
@@ -137,11 +144,38 @@ class LogoDependencies {
137144
*/
138145
this.callbacks = callbacks || { onStopTurtle: null, onRunTurtle: null };
139146

140-
/**
141-
* Speech synthesis (optional)
142-
* @type {Object|null}
143-
*/
144-
this.meSpeak = meSpeak;
147+
// Audio and utility dependencies
148+
this.instruments =
149+
instruments || (typeof window !== "undefined" ? window.instruments : null);
150+
this.instrumentsFilters =
151+
instrumentsFilters ||
152+
(typeof window !== "undefined" ? window.instrumentsFilters : null);
153+
this.instrumentsEffects =
154+
instrumentsEffects ||
155+
(typeof window !== "undefined" ? window.instrumentsEffects : null);
156+
this.widgetWindows =
157+
widgetWindows || (typeof window !== "undefined" ? window.widgetWindows : null);
158+
this.Singer = Singer || (typeof window !== "undefined" ? window.Singer : null);
159+
this.Tone = Tone || (typeof window !== "undefined" ? window.Tone : null);
160+
this.utils = utils || {
161+
doUseCamera: typeof doUseCamera !== "undefined" ? doUseCamera : null,
162+
doStopVideoCam: typeof doStopVideoCam !== "undefined" ? doStopVideoCam : null,
163+
getIntervalDirection:
164+
typeof getIntervalDirection !== "undefined" ? getIntervalDirection : null,
165+
getIntervalNumber: typeof getIntervalNumber !== "undefined" ? getIntervalNumber : null,
166+
mixedNumber: typeof mixedNumber !== "undefined" ? mixedNumber : null,
167+
rationalToFraction:
168+
typeof rationalToFraction !== "undefined" ? rationalToFraction : null,
169+
getStatsFromNotation:
170+
typeof getStatsFromNotation !== "undefined" ? getStatsFromNotation : null,
171+
delayExecution: typeof delayExecution !== "undefined" ? delayExecution : null,
172+
last: typeof last !== "undefined" ? last : null
173+
};
174+
this.classes = classes || {
175+
Notation: typeof Notation !== "undefined" ? Notation : null,
176+
Synth: typeof Synth !== "undefined" ? Synth : null,
177+
StatusMatrix: typeof StatusMatrix !== "undefined" ? StatusMatrix : null
178+
};
145179
}
146180

147181
/**
@@ -179,7 +213,36 @@ class LogoDependencies {
179213
onStopTurtle: activity.onStopTurtle,
180214
onRunTurtle: activity.onRunTurtle
181215
},
182-
meSpeak: activity.meSpeak
216+
217+
// Pass globals for backward compatibility during migration
218+
instruments: typeof instruments !== "undefined" ? instruments : null,
219+
instrumentsFilters:
220+
typeof instrumentsFilters !== "undefined" ? instrumentsFilters : null,
221+
instrumentsEffects:
222+
typeof instrumentsEffects !== "undefined" ? instrumentsEffects : null,
223+
widgetWindows: typeof window !== "undefined" ? window.widgetWindows : null,
224+
Singer: typeof Singer !== "undefined" ? Singer : null,
225+
Tone: typeof Tone !== "undefined" ? Tone : null,
226+
utils: {
227+
doUseCamera: typeof doUseCamera !== "undefined" ? doUseCamera : null,
228+
doStopVideoCam: typeof doStopVideoCam !== "undefined" ? doStopVideoCam : null,
229+
getIntervalDirection:
230+
typeof getIntervalDirection !== "undefined" ? getIntervalDirection : null,
231+
getIntervalNumber:
232+
typeof getIntervalNumber !== "undefined" ? getIntervalNumber : null,
233+
mixedNumber: typeof mixedNumber !== "undefined" ? mixedNumber : null,
234+
rationalToFraction:
235+
typeof rationalToFraction !== "undefined" ? rationalToFraction : null,
236+
getStatsFromNotation:
237+
typeof getStatsFromNotation !== "undefined" ? getStatsFromNotation : null,
238+
delayExecution: typeof delayExecution !== "undefined" ? delayExecution : null,
239+
last: typeof last !== "undefined" ? last : null
240+
},
241+
classes: {
242+
Notation: typeof Notation !== "undefined" ? Notation : null,
243+
Synth: typeof Synth !== "undefined" ? Synth : null,
244+
StatusMatrix: typeof StatusMatrix !== "undefined" ? StatusMatrix : null
245+
}
183246
});
184247
}
185248

js/__tests__/logo-dependencies.test.js

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,16 @@ describe("Logo with LogoDependencies", () => {
101101
onStopTurtle: jest.fn(),
102102
onRunTurtle: jest.fn()
103103
},
104-
meSpeak: null
104+
105+
instruments: {},
106+
Singer: { setSynthVolume: jest.fn() },
107+
Tone: {},
108+
utils: { last: jest.fn() },
109+
classes: {
110+
Notation: jest.fn(),
111+
Synth: jest.fn(),
112+
StatusMatrix: jest.fn()
113+
}
105114
};
106115
});
107116

@@ -121,8 +130,7 @@ describe("Logo with LogoDependencies", () => {
121130
saveLocally: jest.fn(),
122131
showBlocksAfterRun: false,
123132
onStopTurtle: jest.fn(),
124-
onRunTurtle: jest.fn(),
125-
meSpeak: null
133+
onRunTurtle: jest.fn()
126134
};
127135

128136
const logo = new Logo(mockActivity);
@@ -134,7 +142,7 @@ describe("Logo with LogoDependencies", () => {
134142
const deps = new LogoDependencies(mockDeps);
135143
const logo = new Logo(deps);
136144
logo.activity.errorMsg("Test error");
137-
expect(mockDeps.errorHandler).toHaveBeenCalledWith("Test error");
145+
expect(mockDeps.errorHandler).toHaveBeenCalledWith("Test error", undefined);
138146
});
139147

140148
test("Config properties work with getters/setters", () => {
@@ -143,6 +151,12 @@ describe("Logo with LogoDependencies", () => {
143151
logo.activity.showBlocksAfterRun = true;
144152
expect(mockDeps.config.showBlocksAfterRun).toBe(true);
145153
});
154+
155+
test("Audio dependencies are correctly injected", () => {
156+
const logo = new Logo(mockDeps);
157+
expect(logo.deps.instruments).toBe(mockDeps.instruments);
158+
expect(logo.deps.Singer).toBe(mockDeps.Singer);
159+
});
146160
});
147161

148162
describe("LogoDependencies.fromActivity", () => {
@@ -156,12 +170,13 @@ describe("LogoDependencies.fromActivity", () => {
156170
saveLocally: jest.fn(),
157171
showBlocksAfterRun: false,
158172
onStopTurtle: jest.fn(),
159-
onRunTurtle: jest.fn(),
160-
meSpeak: null
173+
onRunTurtle: jest.fn()
161174
};
162175

163176
const deps = LogoDependencies.fromActivity(mockActivity);
164177
expect(deps.blocks).toBe(mockActivity.blocks);
165178
expect(deps.turtles).toBe(mockActivity.turtles);
179+
expect(deps.instruments).toBe(global.instruments);
180+
expect(deps.Singer).toBe(global.Singer);
166181
});
167182
});

js/__tests__/logo.test.js

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,7 @@ describe("Logo Class", () => {
234234
},
235235
onStopTurtle: jest.fn(),
236236
onRunTurtle: jest.fn(),
237-
meSpeak: {
238-
speak: jest.fn()
239-
},
237+
240238
errorMsg: jest.fn(),
241239
saveLocally: jest.fn(),
242240
hideMsgs: jest.fn(),
@@ -371,23 +369,6 @@ describe("Logo Class", () => {
371369
});
372370
});
373371

374-
describe("processSpeak", () => {
375-
test("filters text to only valid characters", () => {
376-
// The code uses this._meSpeak
377-
logo._meSpeak = { speak: jest.fn() };
378-
logo.processSpeak("Hello, World! 123");
379-
380-
expect(logo._meSpeak.speak).toHaveBeenCalledWith("Hello, World ");
381-
});
382-
383-
test("handles empty string", () => {
384-
logo._meSpeak = { speak: jest.fn() };
385-
logo.processSpeak("");
386-
387-
expect(logo._meSpeak.speak).toHaveBeenCalledWith("");
388-
});
389-
});
390-
391372
describe("setTurtleListener", () => {
392373
test("adds event listener to stage", () => {
393374
const listener = jest.fn();

0 commit comments

Comments
 (0)