Github user takezoe commented on a diff in the pull request:

    
https://github.com/apache/incubator-predictionio/pull/412#discussion_r127857163
  
    --- Diff: 
tools/src/main/scala/org/apache/predictionio/tools/commands/Engine.scala ---
    @@ -262,6 +263,56 @@ object Engine extends EitherLogging {
         }
       }
     
    +  /** Batch predict with an engine.
    +    *
    +    * @param ea An instance of [[EngineArgs]]
    +    * @param engineInstanceId An instance of [[engineInstanceId]]
    +    * @param batchPredictArgs An instance of [[BatchPredictArgs]]
    +    * @param sparkArgs An instance of [[SparkArgs]]
    +    * @param pioHome [[String]] with a path to PIO installation
    +    * @param verbose A [[Boolean]]
    +    * @return An instance of [[Expected]] contaning either [[Left]]
    +    *         with an error message or [[Right]] with a handle to process
    +    *         of a running angine  and a function () => Unit,
    +    *         that must be called when the process is complete
    +    */
    +  def batchPredict(
    +    ea: EngineArgs,
    +    engineInstanceId: Option[String],
    +    batchPredictArgs: BatchPredictArgs,
    +    sparkArgs: SparkArgs,
    +    pioHome: String,
    +    verbose: Boolean = false): Expected[(Process, () => Unit)] = {
    +
    +    val engineDirPath = getEngineDirPath(ea.engineDir)
    +    val verifyResult = Template.verifyTemplateMinVersion(
    +      new File(engineDirPath, "template.json"))
    +    if (verifyResult.isLeft) {
    +      return Left(verifyResult.left.get)
    --- End diff --
    
    Generally, avoiding return is preferred in Scala if it's possible. I think 
following is better:
    
    ```scala
    verifyResult match {
      case x @ Left(err) => x
      case Right(_) => {
        ...
      }
    }
    ```
    
    or this is bit shorter:
    
    ```scala
    verifyResult.right.flatMap { _ =>
      ...
    }
    ```
    
    However, there are some other `return`s in this file, so it might be better 
to fully refactor in an another pull request.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to