A better OOP way for solution

Question

I am implementing a functionality in Scala to Copy files from one FileSystem to another FileSystem.

And this is what I have implemented.

CopyFile.scala Methods are empty. However, they will just call external library and will do error handling.

object CopyFiles {
 def localToHDFS(src: String, dest: String): Unit = {
 }
 def S3ToS3(src: String, dest: String): Unit = {
 }
 def localToS3(src: String, des: String): Unit = {
 }
 def HDFSToHDFS(src: String, dest: String): Unit = {
 }
}

Main.scala

import java.util.Properties
import scala.io.Source
object MainClass {
 def main(args: Array[String]): Unit = {
 val propFileURI = "test.properties"
 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")
 if(!srcPath.contains(":") && destPath.toLowerCase().contains("hdfs")){
 CopyFiles.localToHDFS(srcPath, destPath)
 }
 if(!srcPath.toLowerCase().contains(":") && destPath.toLowerCase().contains("s3")){
 CopyFiles.localToS3(srcPath, destPath)
 }
 if(srcPath.toLowerCase().contains("s3") && destPath.toLowerCase().contains("s3")){
 CopyFiles.S3ToS3(srcPath, destPath)
 }
 if(srcPath.toLowerCase().contains("hdfs") && destPath.toLowerCase().contains("hdfs")){
 CopyFiles.HDFSToHDFS(srcPath, destPath)
 }
 }
}

• Is there a better OOP way to solve this?
• Would that be a good idea to write a function which returns the appropriate function based on source and destination location or based on property file to hide complexity from client code?

• Gaurang Shah: downvote without a explanation is not a good stackoverflow practice. if something is missing not appropriate, first point it out.
• greybeard: (then again, this isn't stackoverflow.)
• Mast: It looks like those methods shouldn't be empty. Please provide all relevant code and take a look at the help center.

Answer 1

Single Responsibility

You are doing a few things in main:

• Read srcPath and destPath from the properties file
• Understand which service is responsible for each path
• Find the relevant copy function
• Call the copy function

I suggest creating a method for each of the above bullets.

Polymorphism

You asked

Is there a better OOP way to solve this?

I guess you mean Polymorphism as it one of the key features of OOP.

One way is to write a function that returns the relevant function. Scala support functions types very well.

Another way is creating a trait and classes that implement it (as you did the question you deleted) and a function or factory object that return the relevant class. This way is more common in OOP. Also, it is better in terms of single responsibility:CopyFiles do a lot of different types of copies.

Avoid Ifs

In the question you deleted, you asked if you can rid of the ifs. when the input is the services names(and not the paths), I believe you can use pattern matching easily.

Answer 2

I don't see OO design or code presented: nothing to review here.

The code presented is lacking a description of what it is to accomplish.
main() repeats destPath.toLowerCase() and srcPath.toLowerCase().

 /** True if both parameters specified first contain required. */
 def common(a: String, b: String, required: String): Boolean = {
 a.contains(required) && b.contains(required)
 }
...
 val destLower = destPath.toLowerCase()
 if(!srcPath.contains(":") {
 if (destLower.contains("hdfs")) {
 CopyFiles.localToHDFS(srcPath, destPath)
 } else if (destLower.contains("s3")) {
 CopyFiles.localToS3(srcPath, destPath)
 }
 } else {
 val srcLower = srcPath.toLowerCase()
 if (common(srcLower, destLower, "s3")) {
 CopyFiles.S3ToS3(srcPath, destPath)
 } else if (common(srcLower, destLower, "hdfs")) {
 CopyFiles.HDFSToHDFS(srcPath, destPath)
 }
 }

• greybeard: (I don't know Scala.)