Promisification #169
Promisification #169
Conversation
…e return type to Promise
| /** | ||
| * Here add new restaurant for given location | ||
| */ | ||
| const restaurants: RestaurantMap = new Map([ |
There was a problem hiding this comment.
extracted here the restaurant config in order to better type it + contributors have it easily accessible and separated from other configuration
| ]] | ||
| ]) | ||
| } | ||
| export const config: IConfig = { |
There was a problem hiding this comment.
thanks to moving from Class to constant this config doesn't need to be instantiated (like it was in app.ts) and another benefit is that it can be imported and referenced by any other file/class/service.
| @@ -0,0 +1,20 @@ | |||
| export async function promiseWithTimeout<T>(timeoutMs: number, promise: () => Promise<T>): Promise<T> { | |||
There was a problem hiding this comment.
Handy thing that will handle timeouts when promise is executed... it is used to time out the parsing operation when it takes too long.
| import { config } from "../config"; | ||
|
|
||
| export class HtmlScraperService { | ||
| static async scrape(url: string): Promise<string> { |
There was a problem hiding this comment.
I extracted logic for scraping into this service. Its sole purpose is for given URL return the scraped data.
There was a problem hiding this comment.
I suggest injecting config through ctor, otherwise this in untestable (unmockable)
|
|
||
| export type DocumentType = "pdf" | "image" | "encoded"; | ||
|
|
||
| export class OcrService { |
There was a problem hiding this comment.
Similar to HtmlScraperService... extracted logic for OCR stuff into this service... now it can be better unit tested and also reused for other parsers.
|
|
||
| constructor(private readonly _config: IConfig, private readonly _cache: NodeCache) { } | ||
| private readonly cache = new NodeCache({ checkperiod: (config.cache.expirationTime / 2) }); | ||
| private readonly pendingTasks = new Map<string, Promise<IMenuItem[]>>(); |
There was a problem hiding this comment.
this goes against dependency injection, now the dependencies (config and cache) are created by MenuFetcher itself, which hides them from outside world and makes maintenenc and unittesting harder (if we would have some unittests for MenuFetcher 😄 ) - I suggest injecting those dependencies through ctor as it was originally
| import { config } from "../config"; | ||
|
|
||
| export class HtmlScraperService { | ||
| static async scrape(url: string): Promise<string> { |
There was a problem hiding this comment.
I suggest injecting config through ctor, otherwise this in untestable (unmockable)
|
|
||
| export type DocumentType = "pdf" | "image" | "encoded"; | ||
|
|
||
| export class OcrService { |
Next time I want to check for better options in debugging, watching (realtime reload) etc.