Refactor: tighten output path assembling function signatures (#493)

emmercm · Jul 23, 2023 · 07f9c92 · 07f9c92
1 parent 2cccd42
commit 07f9c92
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 75 deletions.
diff --git a/src/types/options.ts b/src/types/options.ts
@@ -529,7 +529,7 @@ export default class Options implements OptionsProps {
    * Get the output dir, only resolving any tokens.
    */
   getOutputDirParsed(
-    dat?: DAT,
+    dat: DAT,
     inputRomPath?: string,
     game?: Game,
     release?: Release,
@@ -551,20 +551,23 @@ export default class Options implements OptionsProps {
 
   /**
    * Get the full output path for a ROM file.
+   *
+   * @param dat the {@link DAT} that the ROM/{@link Game} is from.
+   * @param inputRomPath the input file's full file path.
+   * @param game the {@link Game} that this file matches to.
+   * @param release a {@link Release} from the {@link Game}.
+   * @param romFilename the intended output filename (including extension).
    */
   getOutputFileParsed(
-    dat?: DAT,
-    inputRomPath?: string,
-    game?: Game,
-    release?: Release,
-    romFilename?: string,
+    dat: DAT,
+    inputRomPath: string,
+    game: Game,
+    release: Release | undefined,
+    romFilename: string,
   ): string {
-    let romFilenameSanitized: string | undefined;
-    if (romFilename) {
-      romFilenameSanitized = romFilename.replace(/[\\/]/g, path.sep);
-      if (!dat?.getRomNamesContainDirectories()) {
-        romFilenameSanitized = romFilenameSanitized.replace(/[\\/]/g, '_');
-      }
+    let romFilenameSanitized = romFilename.replace(/[\\/]/g, path.sep);
+    if (!dat?.getRomNamesContainDirectories()) {
+      romFilenameSanitized = romFilenameSanitized.replace(/[\\/]/g, '_');
     }
 
     let output = this.getOutputDirParsed(dat, inputRomPath, game, release, romFilename);
@@ -578,7 +581,7 @@ export default class Options implements OptionsProps {
       output = path.join(output, mirroredDir);
     }
 
-    if (dat && this.getDirDatName()) {
+    if (this.getDirDatName() && dat.getNameShort()) {
       output = path.join(output, dat.getNameShort());
     }
 
@@ -590,8 +593,7 @@ export default class Options implements OptionsProps {
       output = path.join(output, letter);
     }
 
-    if (game
-      && game.getRoms().length > 1
+    if (game.getRoms().length > 1
       && (!romFilenameSanitized || !FileFactory.isArchive(romFilenameSanitized))
     ) {
       output = path.join(output, game.getName());
@@ -606,7 +608,7 @@ export default class Options implements OptionsProps {
 
   private static replaceTokensInOutputPath(
     outputPath: string,
-    dat?: DAT,
+    dat: DAT,
     inputRomPath?: string,
     release?: Release,
     outputRomFilename?: string,
@@ -626,11 +628,7 @@ export default class Options implements OptionsProps {
     return result;
   }
 
-  private static replaceDatTokens(input: string, dat?: DAT): string {
-    if (!dat) {
-      return input;
-    }
-
+  private static replaceDatTokens(input: string, dat: DAT): string {
     return input.replace('{datName}', dat.getName().replace(/[\\/]/g, '_'));
   }
 

diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts
@@ -6,6 +6,7 @@ import LogLevel from '../../src/console/logLevel.js';
 import Constants from '../../src/constants.js';
 import ArgumentsParser from '../../src/modules/argumentsParser.js';
 import DAT from '../../src/types/logiqx/dat.js';
+import Game from '../../src/types/logiqx/game.js';
 import Header from '../../src/types/logiqx/header.js';
 
 const dummyRequiredArgs = ['--input', os.devNull, '--output', os.devNull];
@@ -219,17 +220,23 @@ describe('options', () => {
   });
 
   it('should parse "output"', () => {
+    const dummyDat = new DAT(new Header(), []);
+    const dummyInputRomPath = '';
+    const dummyGame = new Game();
+    const dummyRelease = undefined;
+    const dummyRomFilename = '';
+
     // Test requirements per command
     expect(() => argumentsParser.parse(['test'])).toThrow(/missing required argument/i);
     expect(() => argumentsParser.parse(['copy', '--input', os.devNull])).toThrow(/missing required option/i);
     expect(() => argumentsParser.parse(['move', '--input', os.devNull])).toThrow(/missing required option/i);
     expect(() => argumentsParser.parse(['copy', 'zip', '--input', os.devNull])).toThrow(/missing required option/i);
     expect(() => argumentsParser.parse(['copy', 'clean', '--input', os.devNull])).toThrow(/missing required option/i);
-    expect(argumentsParser.parse(['report', '--dat', os.devNull, '--input', os.devNull]).getOutputFileParsed()).toContain(Constants.GLOBAL_TEMP_DIR);
+    expect(argumentsParser.parse(['report', '--dat', os.devNull, '--input', os.devNull]).getOutputFileParsed(dummyDat, dummyInputRomPath, dummyGame, dummyRelease, dummyRomFilename)).toContain(Constants.GLOBAL_TEMP_DIR);
     // Test value
-    expect(argumentsParser.parse(['copy', '--input', os.devNull, '-o', 'foo']).getOutputFileParsed()).toEqual('foo');
-    expect(argumentsParser.parse(['copy', '--input', os.devNull, '--output', 'foo']).getOutputFileParsed()).toEqual('foo');
-    expect(argumentsParser.parse(['copy', '--input', os.devNull, '--output', 'foo', '--output', 'bar']).getOutputFileParsed()).toEqual('bar');
+    expect(argumentsParser.parse(['copy', '--input', os.devNull, '-o', 'foo']).getOutputFileParsed(dummyDat, dummyInputRomPath, dummyGame, dummyRelease, dummyRomFilename)).toEqual('foo');
+    expect(argumentsParser.parse(['copy', '--input', os.devNull, '--output', 'foo']).getOutputFileParsed(dummyDat, dummyInputRomPath, dummyGame, dummyRelease, dummyRomFilename)).toEqual('foo');
+    expect(argumentsParser.parse(['copy', '--input', os.devNull, '--output', 'foo', '--output', 'bar']).getOutputFileParsed(dummyDat, dummyInputRomPath, dummyGame, dummyRelease, dummyRomFilename)).toEqual('bar');
   });
 
   it('should parse "dir-mirror"', () => {