CODEWITHSUNDEEP
scalaclass
Gaurang Shah
Gaurang Shah

Reputation: 12920

Scala - Implement Object Factory Pattern

I am trying to implement object factory design pattern in scala. However I am not able to undertand how to return the object based on condition.

I tried to return Option however it's not working as expected. 

import java.io.File
import java.util.Properties

import scala.io.Source

abstract class FileSystem {
  def moveFile(propFileURI: String): Unit
}

object FileSystem {

  private class HDFSystem extends FileSystem{
    override def moveFile(propFileURI: String): Unit = {
      println(" HDFS  move file")
    }
  }

  private class S3System extends FileSystem {
    override def moveFile(propFileURI: String): Unit = {
      println("S3 Move File ")
    }
  }

  // How to handle this??
  def apply(propFileURI: String): Option[FileSystem] = {
    val properties: Properties = new Properties()

    val source = Source.fromFile( System.getProperty("user.dir")+"\\src\\main\\resources\\"+propFileURI).reader
    properties.load(source)
    val srcPath = properties.getProperty("srcPath")
    val destPath = properties.getProperty("destPath")

    var Obj = None:Option[FileSystem]

    if (destPath.contains("hdfs")){
      Obj = Option(new HDFSystem())
    }
    if (srcPath.contains("s3") && destPath.contains("s3")){
     Obj = Option(new S3System())
    }
    Obj
  }

  def main(args: Array[String]): Unit = {
    val obj = FileSystem("test.properties")
    obj match {
      case test: FileSystem => test.moveFile("test.properties")
      case None => None
    }

  }
}

How to handle the Apply method based on condition I have mentioned? Do I really need to return option ?

Upvotes: 1

Views: 166

Answers (2)

Mario Galic
Mario Galic

Reputation: 48420

There are ways to clean up your implementation, for example, usage of var could be avoided

if (destPath.contains("hdfs"))
  Some(new HDFSystem())
else if (srcPath.contains("s3") && destPath.contains("s3")) 
  Some(new S3System())
else 
  None

however as far as I understand the main point of the question is

Do I really need to return Option?

This is a design decision with tradeoffs and no clear answer. Ask yourself how should the system react if .properties file is missing the appropriate configuration key:

  • Could we construct a meaningful default FileSystem object? In this case there is no need for Option, just return the default and continue processing.
  • If we cannot construct a meaningful default, is there any point in continuing? Should the system crash? In which case we might throw.
  • If the system can continue operating despite not being able to construct FileSystem, then we could model this information as Option or Either etc., perhaps log the event, and continue processing.

These are just some considerations to take into account. Personally, I have found that a misconfigured system is hard to recover from.

Upvotes: 2

Mateusz Kubuszok
Mateusz Kubuszok

Reputation: 27535

Options aren't Java's @nullables. You pattern match on Some if value is present:

    val obj = FileSystem("test.properties") // Option[FileSystem]
    obj match {
      case Some(test) => test.moveFile("test.properties")
      case None       =>
    }

Also in Scala almost everything is an expression so if-else, blocks, loops and functions returns their last value as the value of all. Also we should take IO errors into consideration, so Try could be better than Option:

  def apply(propFileURI: String): Try[FileSystem] =
    Try {
      // reading properties could fail
      val p = new Properties()
      val source = Source.fromFile( System.getProperty("user.dir")+"\\src\\main\\resources\\"+propFileURI).reader
      p.load(source)
      p
    }.flatMap { properties =>
      // reading from properties could fail
      val srcPath = properties.getProperty("srcPath")
      val destPath = properties.getProperty("destPath")

      if (destPath.contains("hdfs")) Success(new HDFSystem())
      else if (srcPath.contains("s3") && destPath.contains("s3")) Success(new S3System())
      else Failure(new Exception("Unable to recognize filesystem"))
    }

  def main(args: Array[String]): Unit =
    FileSystem("test.properties") match {
      case Success(fileSystem) => fileSystem.moveFile("test.properties")
      case Failure(error)      => error.printStackTrace()
    }

We could convert Try at any moment to Option with .toOption and pattern-match on it. But this way we have no information on error. We could also create a FileSystemError type for storing error information and return Either[FileSystemError, FileSystem] instead of Try. Lastly, we could just throw Exception, but this way we return to Java-like practices that doesn't tell us that error can happen, and it surprises us at runtime.

What I would surely do is to rename apply - we usually expect object's apply to always return success, so if it isn't possible and we use some sort of smart constructor we should give it some other name. E.g. here we could name it

def resolveFor(propFileURI: String): Either[FileSystemError, FileSystem] = ...

so that everyone using it would know what behavior expect just from the signature.

Upvotes: 1

Related Questions