0

Say I have the following classes:

abstract class AbstractFileReader {
  // ???
}

class SyncFileReader extends AbstractFileReader {
  public readFileDecorated(filePath: string): string {
    console.log('Filepath: ');
    console.log(filePath);
    const contents = this.readFileInternal(filePath);
    console.log('Length of file: ');
    console.log(contents.length);
    return contents;
  }
  private readFileInternal(filePath: string): string {
    return fs.readFileSync(filePath, {'encoding': 'utf8'});
  }
}

class AsyncFileReader extends AbstractFileReader {
  // highly redundant code...
  public async readFileDecorated(filePath: string): Promise<string> {
    console.log('Filepath: ');
    console.log(filePath);
    const contents = await this.readFileInternal(filePath);
    console.log('Length of file: ');
    console.log(contents.length);
    return contents;    
  }
  private async readFileInternal(filePath: string): Promise<string> {
    return await fs.promises.readFile(filePath, {'encoding': 'utf8'});
  }
}

const syncFileReader = new SyncFileReader();
const asyncFileReader = new AsyncFileReader();
asyncFileReader.readFileDecorated('./test.txt').then((contents) => {
  console.log(contents);
}).catch((reason) => console.log('abc'));

// The following call should still work without change after the changes in AbstractFileReader.
console.log(syncFileReader.readFileDecorated('./test.txt'));

The code in readFileDecorated (just a silly example of course) is highly redundant, so I want to put it in a method in AbstractFileReader instead. However, the problem is that readFileDecorated is sync in SyncFileReader but async in AsyncFileReader.

The straightforward solution I came up with was to make everything async in AbstractFileReader. This would work, but then, the call in the last line had to be changed, which I do not want to do because a SyncFileReader should expose only sync syntax.

Another solution was to use readFileDecoratedPre(filePath) and readFileDecoratedPost(contents) methods that are called before or after (resp.) the call to readFileInternal, but this would not be a feasible solution when a method contains several sync/async calls.

Remirror
  • 692
  • 5
  • 14

2 Answers2

1

You can make synchronous code asynchronous by using Promises. You can create a promise and immediately resolve it.

This way the signatures in your SyncFileReader are the same as in AsyncFileReader.

class SyncFileReader extends AbstractFileReader {
  public readFileDecorated(filePath: string): Promise<string> {
    console.log('Filepath: ');
    console.log(filePath);
    const contents = this.readFileInternal(filePath);
    console.log('Length of file: ');
    console.log(contents.length);
    return new Promise((resolve) => resolve(contents));
  }
  private readFileInternal(filePath: string): Promise<string> {
    return new Promise((resolve) => resolve(fs.readFileSync(filePath, {'encoding': 'utf8'})));
  }
}

You could also check if the value returned from a method is a Promise and await it if it is.

const promiseContents: string|Promise<string> = this.readFileInternal(filePath);
let contents: string;
if (typeof contents?.then === 'function') {
    contents = await promiseContents
} else {
    contents = promiseContents
}

But this is not the nicest solution.

Qurben
  • 1,276
  • 1
  • 10
  • 21
  • Thanks, but the last call `console.log(syncFileReader.readFileDecorated('./test.txt'));` would then log the Promise object, not the string. I do not want to change the sync syntax of `SyncFileReader`. – Remirror Sep 01 '20 at 09:40
  • If you know you'll get a `SyncFileReader` and want to keep it synchronous you should not try to generalize this code, maybe pull common code in functions in a super class or utility file. Making this a general solution and forcing async methods to be sync is really bad practice [see here](https://stackoverflow.com/questions/9121902/call-an-asynchronous-javascript-function-synchronously) – Qurben Sep 01 '20 at 09:54
0

I now went for the following solution:

First, I created a function handleValue like this:

function handleValue<T>(sync: boolean, valueOrPromise: T | Promise<T>, callback: (t: T) => void): T | Promise<T> {
  if (sync) {
    const value = valueOrPromise as T;
    callback(value);
    return value;
  } else {
    const promise = valueOrPromise as Promise<T>;
    promise.then(callback);
    return promise;
  }  
}

Then, I changed the classes as follows (note that I changed the method signatures slightly):

abstract class AbstractFileReader {
  protected _readFileDecorated(filePath: string, sync: boolean): string | Promise<string> {
    console.log('Filepath: ');
    console.log(filePath);
    const contentsOrPromise = this._readFileInternal(filePath);
    const callback = (contents: string): void => {
      console.log('Length of file: ');
      console.log(contents.length);
    };
    handleValue(sync, contentsOrPromise, callback);
    return contentsOrPromise;
 }

 protected abstract _readFileInternal(filePath: string): string | Promise<string>;
}

class SyncFileReader extends AbstractFileReader {
  public readFileDecorated(filePath: string): string {
    return this._readFileDecorated(filePath, true) as string;
  }
  protected _readFileInternal(filePath: string): string {
    return fs.readFileSync(filePath, {'encoding': 'utf8'});
  }
}

class AsyncFileReader extends AbstractFileReader {
  public async readFileDecorated(filePath: string): Promise<string> {
    return this._readFileDecorated(filePath, false);
  }
  protected async _readFileInternal(filePath: string): Promise<string> {
    return await fs.promises.readFile(filePath, {'encoding': 'utf8'});
  }
}

I admit that this is a bit of cheating because the callback function in _readFileDecorated is essentially the same idea as readFileDecoratedPost(contents), but at least it is better than the original solution because it has no redundancy anymore.

Remirror
  • 692
  • 5
  • 14