Refactor Operations

Author: sequba

src/DependencyGraph/DependencyGraph.ts

@@ -659,17 +659,6 @@ export class DependencyGraph {
     }
   }
 
-  /**
-   * Iterator over all formula nodes in the graph.
-   */
-  public* formulaNodes(): IterableIterator<FormulaVertex> {
-    for (const vertex of this.graph.getNodes()) {
-      if (vertex instanceof FormulaVertex) {
-        yield vertex
-      }
-    }
-  }
-
   public* entriesFromRowsSpan(rowsSpan: RowsSpan): IterableIterator<[SimpleCellAddress, CellVertex]> {
     yield* this.addressMapping.entriesFromRowsSpan(rowsSpan)
   }
@@ -686,24 +675,32 @@ export class DependencyGraph {
     return this.addressMapping.getCell(address)
   }
 
+  /**
+   * Checks if the given sheet ID refers to a placeholder sheet (doesn't exist but is referenced by other sheets)
+   */
+  private isPlaceholder(sheetId: number): boolean {
+    return sheetId !== NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS &&
+      !this.sheetMapping.hasSheetWithId(sheetId, { includePlaceholders: false })
+  }
+
   public getCellValue(address: SimpleCellAddress): InterpreterValue {
-    if (address.sheet !== NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS && !this.sheetMapping.hasSheetWithId(address.sheet, { includePlaceholders: false })) {
+    if (this.isPlaceholder(address.sheet)) {
       return new CellError(ErrorType.REF, ErrorMessage.SheetRef)
     }
 
     return this.addressMapping.getCellValue(address)
   }
 
   public getRawValue(address: SimpleCellAddress): RawCellContent {
-    if (address.sheet !== NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS && !this.sheetMapping.hasSheetWithId(address.sheet, { includePlaceholders: false })) {
+    if (this.isPlaceholder(address.sheet)) {
       return null
     }
 
     return this.addressMapping.getRawValue(address)
   }
 
   public getScalarValue(address: SimpleCellAddress): InternalScalarValue {
-    if (address.sheet !== NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS && !this.sheetMapping.hasSheetWithId(address.sheet, { includePlaceholders: false })) {
+    if (this.isPlaceholder(address.sheet)) {
       return new CellError(ErrorType.REF, ErrorMessage.SheetRef)
     }
 
@@ -1258,7 +1255,7 @@ export class DependencyGraph {
 
     const hasDependentInExistingSheet = dependents.some(addr => {
       if (isSimpleCellAddress(addr)) {
-        return addr.sheet === NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS || this.sheetMapping.hasSheetWithId(addr.sheet, { includePlaceholders: false })
+        return !this.isPlaceholder(addr.sheet)
       }
 
       if (isSimpleCellRange(addr)) {

src/Operations.ts

@@ -217,17 +217,26 @@ export class Operations {
     return columnsRemovals
   }
 
+  /**
+   * Clears the sheet content.
+   */
   public clearSheet(sheetId: number) {
     this.dependencyGraph.clearSheet(sheetId)
     this.columnSearch.removeSheet(sheetId)
   }
 
+  /**
+   * Adds a new sheet to the workbook.
+   */
   public addSheet(name?: string): string {
     const sheetId = this.sheetMapping.addSheet(name)
     this.dependencyGraph.addSheet(sheetId)
     return this.sheetMapping.getSheetNameOrThrowError(sheetId)
   }
 
+  /**
+   * Removes a sheet from the workbook.
+   */
   public removeSheet(sheetId: number): { version: number, scopedNamedExpressions: [InternalNamedExpression, ClipboardCell][] } {
     this.dependencyGraph.removeSheet(sheetId)
     this.columnSearch.removeSheet(sheetId)
@@ -237,12 +246,18 @@ export class Operations {
     return { version: 0, scopedNamedExpressions }
   }
 
+  /**
+   * Removes a sheet from the workbook by name.
+   */
   public removeSheetByName(sheetName: string) {
     const sheetId = this.sheetMapping.getSheetIdOrThrowError(sheetName)
     return this.removeSheet(sheetId)
   }
 
-  public renameSheet(sheetId: number, newName: string): Maybe<string> { // TODO: refactor
+  /**
+   * Renames a sheet in the workbook.
+   */
+  public renameSheet(sheetId: number, newName: string): Maybe<string> {
     const { previousDisplayName, mergedSheetWith } = this.sheetMapping.renameSheet(sheetId, newName)
 
     if (mergedSheetWith !== undefined) {