dermasmid commented on code in PR #29944:
URL: https://github.com/apache/beam/pull/29944#discussion_r1451597194


##########
sdks/typescript/src/apache_beam/runners/runner.ts:
##########
@@ -64,21 +64,22 @@ export class PipelineResult {
  * transforms, non-trivial windowing, etc. are used).
  */
 export function createRunner(options: any = {}): Runner {
-  let runnerConstructor: (any) => Runner;
   if (options.runner === undefined || options.runner === "default") {
-    runnerConstructor = defaultRunner;
-  } else if (options.runner === "direct") {
-    runnerConstructor = require("./direct_runner").directRunner;
-  } else if (options.runner === "universal") {
-    runnerConstructor = require("./universal").universalRunner;
-  } else if (options.runner === "flink") {
-    runnerConstructor = require("./flink").flinkRunner;
-  } else if (options.runner === "dataflow") {
-    runnerConstructor = require("./dataflow").dataflowRunner;
-  } else {
-    throw new Error("Unknown runner: " + options.runner);
+    return defaultRunner(options);
   }
-  return runnerConstructor(options);
+  if (options.runner === "direct") {

Review Comment:
   it thought it will be more readable this way, but having both the else if, 
and the return would be even less readable.
   the "early return" is a common pattern in JS, but i dont feel strongly about 
it, and can revert if you want 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to