Skip to content

Commit 02e64be

Browse files
gonfunkogonfunko
authored
refactor: make VariableMap implement IVariableMap. (#8395)
* refactor: make VariableMap implement IVariableMap. * chore: remove unused arrayUtils import. * chore: fix comment on variable map backing store. * chore: Added JSDoc to new VariableMap methods. * chore: Improve test descriptions.
1 parent 107403b commit 02e64be

File tree

4 files changed

+164
-71
lines changed

4 files changed

+164
-71
lines changed

‎core/variable_map.ts‎

Lines changed: 87 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -19,34 +19,35 @@ import './events/events_var_rename.js';
1919
import type {Block} from './block.js';
2020
import * as dialog from './dialog.js';
2121
import * as eventUtils from './events/utils.js';
22+
import * as registry from './registry.js';
2223
import {Msg} from './msg.js';
2324
import {Names} from './names.js';
24-
import * as arrayUtils from './utils/array.js';
2525
import * as idGenerator from './utils/idgenerator.js';
2626
import {VariableModel} from './variable_model.js';
2727
import type {Workspace} from './workspace.js';
28+
import type {IVariableMap} from './interfaces/i_variable_map.js';
2829

2930
/**
3031
* Class for a variable map. This contains a dictionary data structure with
3132
* variable types as keys and lists of variables as values. The list of
3233
* variables are the type indicated by the key.
3334
*/
34-
export class VariableMap {
35+
export class VariableMap implements IVariableMap<VariableModel> {
3536
/**
36-
* A map from variable type to list of variable names. The lists contain
37+
* A map from variable type to map of IDs to variables. The maps contain
3738
* all of the named variables in the workspace, including variables that are
3839
* not currently in use.
3940
*/
40-
private variableMap = new Map<string, VariableModel[]>();
41+
private variableMap = new Map<string, Map<string, VariableModel>>();
4142

4243
/** @param workspace The workspace this map belongs to. */
4344
constructor(public workspace: Workspace) {}
4445

4546
/** Clear the variable map. Fires events for every deletion. */
4647
clear() {
4748
for (const variables of this.variableMap.values()) {
48-
while (variables.length > 0) {
49-
this.deleteVariable(variables[0]);
49+
for (const variable of variables.values()) {
50+
this.deleteVariable(variable);
5051
}
5152
}
5253
if (this.variableMap.size !== 0) {
@@ -60,10 +61,10 @@ export class VariableMap {
6061
*
6162
* @param variable Variable to rename.
6263
* @param newName New variable name.
63-
* @internal
64+
* @returns The newly renamed variable.
6465
*/
65-
renameVariable(variable: VariableModel, newName: string) {
66-
if (variable.name === newName) return;
66+
renameVariable(variable: VariableModel, newName: string): VariableModel {
67+
if (variable.name === newName) return variable;
6768
const type = variable.type;
6869
const conflictVar = this.getVariable(newName, type);
6970
const blocks = this.workspace.getAllBlocks(false);
@@ -87,6 +88,20 @@ export class VariableMap {
8788
} finally {
8889
eventUtils.setGroup(existingGroup);
8990
}
91+
return variable;
92+
}
93+
94+
changeVariableType(variable: VariableModel, newType: string): VariableModel {
95+
this.variableMap.get(variable.getType())?.delete(variable.getId());
96+
variable.setType(newType);
97+
const newTypeVariables =
98+
this.variableMap.get(newType) ?? new Map<string, VariableModel>();
99+
newTypeVariables.set(variable.getId(), variable);
100+
if (!this.variableMap.has(newType)) {
101+
this.variableMap.set(newType, newTypeVariables);
102+
}
103+
104+
return variable;
90105
}
91106

92107
/**
@@ -159,8 +174,8 @@ export class VariableMap {
159174
}
160175
// Finally delete the original variable, which is now unreferenced.
161176
eventUtils.fire(new (eventUtils.get(eventUtils.VAR_DELETE))(variable));
162-
// And remove it from the list.
163-
arrayUtils.removeElem(this.variableMap.get(type)!, variable);
177+
// And remove it from the map.
178+
this.variableMap.get(type)?.delete(variable.getId());
164179
}
165180

166181
/* End functions for renaming variables. */
@@ -177,8 +192,8 @@ export class VariableMap {
177192
*/
178193
createVariable(
179194
name: string,
180-
opt_type?: string | null,
181-
opt_id?: string | null,
195+
opt_type?: string,
196+
opt_id?: string,
182197
): VariableModel {
183198
let variable = this.getVariable(name, opt_type);
184199
if (variable) {
@@ -204,43 +219,43 @@ export class VariableMap {
204219
const type = opt_type || '';
205220
variable = new VariableModel(this.workspace, name, type, id);
206221

207-
const variables = this.variableMap.get(type) || [];
208-
variables.push(variable);
209-
// Delete the list of variables of this type, and re-add it so that
210-
// the most recent addition is at the end.
211-
// This is used so the toolbox's set block is set to the most recent
212-
// variable.
213-
this.variableMap.delete(type);
214-
this.variableMap.set(type, variables);
215-
222+
const variables =
223+
this.variableMap.get(type) ?? new Map<string, VariableModel>();
224+
variables.set(variable.getId(), variable);
225+
if (!this.variableMap.has(type)) {
226+
this.variableMap.set(type, variables);
227+
}
216228
eventUtils.fire(new (eventUtils.get(eventUtils.VAR_CREATE))(variable));
217229

218230
return variable;
219231
}
220232

233+
/**
234+
* Adds the given variable to this variable map.
235+
*
236+
* @param variable The variable to add.
237+
*/
238+
addVariable(variable: VariableModel) {
239+
const type = variable.getType();
240+
if (!this.variableMap.has(type)) {
241+
this.variableMap.set(type, new Map<string, VariableModel>());
242+
}
243+
this.variableMap.get(type)?.set(variable.getId(), variable);
244+
}
245+
221246
/* Begin functions for variable deletion. */
222247
/**
223248
* Delete a variable.
224249
*
225250
* @param variable Variable to delete.
226251
*/
227252
deleteVariable(variable: VariableModel) {
228-
const variableId = variable.getId();
229-
const variableList = this.variableMap.get(variable.type);
230-
if (variableList) {
231-
for (let i = 0; i < variableList.length; i++) {
232-
const tempVar = variableList[i];
233-
if (tempVar.getId() === variableId) {
234-
variableList.splice(i, 1);
235-
eventUtils.fire(
236-
new (eventUtils.get(eventUtils.VAR_DELETE))(variable),
237-
);
238-
if (variableList.length === 0) {
239-
this.variableMap.delete(variable.type);
240-
}
241-
return;
242-
}
243-
}
253+
const variables = this.variableMap.get(variable.type);
254+
if (!variables || !variables.has(variable.getId())) return;
255+
variables.delete(variable.getId());
256+
eventUtils.fire(new (eventUtils.get(eventUtils.VAR_DELETE))(variable));
257+
if (variables.size === 0) {
258+
this.variableMap.delete(variable.type);
244259
}
245260
}
246261

@@ -321,17 +336,16 @@ export class VariableMap {
321336
* the empty string, which is a specific type.
322337
* @returns The variable with the given name, or null if it was not found.
323338
*/
324-
getVariable(name: string, opt_type?: string | null): VariableModel | null {
339+
getVariable(name: string, opt_type?: string): VariableModel | null {
325340
const type = opt_type || '';
326-
const list = this.variableMap.get(type);
327-
if (list) {
328-
for (let j = 0, variable; (variable = list[j]); j++) {
329-
if (Names.equals(variable.name, name)) {
330-
return variable;
331-
}
332-
}
333-
}
334-
return null;
341+
const variables = this.variableMap.get(type);
342+
if (!variables) return null;
343+
344+
return (
345+
[...variables.values()].find((variable) =>
346+
Names.equals(variable.getName(), name),
347+
) ?? null
348+
);
335349
}
336350

337351
/**
@@ -342,10 +356,8 @@ export class VariableMap {
342356
*/
343357
getVariableById(id: string): VariableModel | null {
344358
for (const variables of this.variableMap.values()) {
345-
for (const variable of variables) {
346-
if (variable.getId() === id) {
347-
return variable;
348-
}
359+
if (variables.has(id)) {
360+
return variables.get(id) ?? null;
349361
}
350362
}
351363
return null;
@@ -361,11 +373,19 @@ export class VariableMap {
361373
*/
362374
getVariablesOfType(type: string | null): VariableModel[] {
363375
type = type || '';
364-
const variableList = this.variableMap.get(type);
365-
if (variableList) {
366-
return variableList.slice();
367-
}
368-
return [];
376+
const variables = this.variableMap.get(type);
377+
if (!variables) return [];
378+
379+
return [...variables.values()];
380+
}
381+
382+
/**
383+
* Returns a list of unique types of variables in this variable map.
384+
*
385+
* @returns A list of unique types of variables in this variable map.
386+
*/
387+
getTypes(): string[] {
388+
return [...this.variableMap.keys()];
369389
}
370390

371391
/**
@@ -399,7 +419,7 @@ export class VariableMap {
399419
getAllVariables(): VariableModel[] {
400420
let allVariables: VariableModel[] = [];
401421
for (const variables of this.variableMap.values()) {
402-
allVariables = allVariables.concat(variables);
422+
allVariables = allVariables.concat(...variables.values());
403423
}
404424
return allVariables;
405425
}
@@ -410,9 +430,13 @@ export class VariableMap {
410430
* @returns All of the variable names of all types.
411431
*/
412432
getAllVariableNames(): string[] {
413-
return Array.from(this.variableMap.values())
414-
.flat()
415-
.map((variable) => variable.name);
433+
const names: string[] = [];
434+
for (const variables of this.variableMap.values()) {
435+
for (const variable of variables.values()) {
436+
names.push(variable.getName());
437+
}
438+
}
439+
return names;
416440
}
417441

418442
/**
@@ -438,3 +462,5 @@ export class VariableMap {
438462
return uses;
439463
}
440464
}
465+
466+
registry.register(registry.Type.VARIABLE_MAP, registry.DEFAULT, VariableMap);

‎core/variables.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,11 @@ function createVariable(
610610
// Create a potential variable if in the flyout.
611611
let variable = null;
612612
if (potentialVariableMap) {
613-
variable = potentialVariableMap.createVariable(opt_name, opt_type, id);
613+
variable = potentialVariableMap.createVariable(
614+
opt_name,
615+
opt_type,
616+
id ?? undefined,
617+
);
614618
} else {
615619
// In the main workspace, create a real variable.
616620
variable = workspace.createVariable(opt_name, opt_type, id);

‎core/workspace.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,11 @@ export class Workspace implements IASTNodeLocation {
400400
opt_type?: string | null,
401401
opt_id?: string | null,
402402
): VariableModel {
403-
return this.variableMap.createVariable(name, opt_type, opt_id);
403+
return this.variableMap.createVariable(
404+
name,
405+
opt_type ?? undefined,
406+
opt_id ?? undefined,
407+
);
404408
}
405409

406410
/**
@@ -456,7 +460,7 @@ export class Workspace implements IASTNodeLocation {
456460
* if none are found.
457461
*/
458462
getVariablesOfType(type: string | null): VariableModel[] {
459-
return this.variableMap.getVariablesOfType(type);
463+
return this.variableMap.getVariablesOfType(type ?? '');
460464
}
461465

462466
/**

0 commit comments

Comments
 (0)