Skip to main content
Commonmark migration
Source Link

Idiomatic Scala Style

  • Class, object, and trait names follow camelCase style with the first letter capitalized. One example where you don't follow this rule is abstract class spMeta extends logging.

  • For a few reasons, it is considered bad practice to user underscores in names. Applying the convention from the bullet point above with this we'd make the following transformation to your code: class sp_01 extends spMeta => class Sp01 extends SpMeta.

  • The above rules also extend to types that you define. Consequently I'd change type inputT to type InputT, etc.

  • Methods which act as accessors should be declared without parenthesis (unless they have side effects). In particular this applies to your def getSolution(): SolutionT = ... which should just be plain old parenthesis-less def getSolution: SolutionT = ...

  • If you ever have any style questions check out this documentation.

Code Suggestions

A modified version of your getSolutionFiles method with explanations below.

    def getSolutionFiles(dir: File): Array[File] = {
        val dirContents = dir.listFiles
        val targetFiles = dirContents filter(x => x.getName.startsWith("sp_") && !x.isDirectory)
        val childDirs   = dirContents filter(_.isDirectory)
        targetFiles ++ childDirs flatMap(d => getSolutionFiles(d))
    }
  • I renamed the method paramter to dir from file.
  • All of the operations in this method are on the container type Array. So I renamed some of the values to remove any mention of List.
  • Overall I think the renaming of these values make this method much easier to to immediately comprehend.
  • Just to show you 'another way' and not because I thought anything was wrong with your original choice, I replaced your use of a regular expression for finding the correct files with startsWith(...). Note there is also a method endsWith(...).
  • A bug could occur if the name of a file passed your filter predicate but is in fact a directory. I added a condition to to eliminate that possibility.

Cheers :)

Idiomatic Scala Style

  • Class, object, and trait names follow camelCase style with the first letter capitalized. One example where you don't follow this rule is abstract class spMeta extends logging.

  • For a few reasons, it is considered bad practice to user underscores in names. Applying the convention from the bullet point above with this we'd make the following transformation to your code: class sp_01 extends spMeta => class Sp01 extends SpMeta.

  • The above rules also extend to types that you define. Consequently I'd change type inputT to type InputT, etc.

  • Methods which act as accessors should be declared without parenthesis (unless they have side effects). In particular this applies to your def getSolution(): SolutionT = ... which should just be plain old parenthesis-less def getSolution: SolutionT = ...

  • If you ever have any style questions check out this documentation.

Code Suggestions

A modified version of your getSolutionFiles method with explanations below.

    def getSolutionFiles(dir: File): Array[File] = {
        val dirContents = dir.listFiles
        val targetFiles = dirContents filter(x => x.getName.startsWith("sp_") && !x.isDirectory)
        val childDirs   = dirContents filter(_.isDirectory)
        targetFiles ++ childDirs flatMap(d => getSolutionFiles(d))
    }
  • I renamed the method paramter to dir from file.
  • All of the operations in this method are on the container type Array. So I renamed some of the values to remove any mention of List.
  • Overall I think the renaming of these values make this method much easier to to immediately comprehend.
  • Just to show you 'another way' and not because I thought anything was wrong with your original choice, I replaced your use of a regular expression for finding the correct files with startsWith(...). Note there is also a method endsWith(...).
  • A bug could occur if the name of a file passed your filter predicate but is in fact a directory. I added a condition to to eliminate that possibility.

Cheers :)

Idiomatic Scala Style

  • Class, object, and trait names follow camelCase style with the first letter capitalized. One example where you don't follow this rule is abstract class spMeta extends logging.

  • For a few reasons, it is considered bad practice to user underscores in names. Applying the convention from the bullet point above with this we'd make the following transformation to your code: class sp_01 extends spMeta => class Sp01 extends SpMeta.

  • The above rules also extend to types that you define. Consequently I'd change type inputT to type InputT, etc.

  • Methods which act as accessors should be declared without parenthesis (unless they have side effects). In particular this applies to your def getSolution(): SolutionT = ... which should just be plain old parenthesis-less def getSolution: SolutionT = ...

  • If you ever have any style questions check out this documentation.

Code Suggestions

A modified version of your getSolutionFiles method with explanations below.

    def getSolutionFiles(dir: File): Array[File] = {
        val dirContents = dir.listFiles
        val targetFiles = dirContents filter(x => x.getName.startsWith("sp_") && !x.isDirectory)
        val childDirs   = dirContents filter(_.isDirectory)
        targetFiles ++ childDirs flatMap(d => getSolutionFiles(d))
    }
  • I renamed the method paramter to dir from file.
  • All of the operations in this method are on the container type Array. So I renamed some of the values to remove any mention of List.
  • Overall I think the renaming of these values make this method much easier to to immediately comprehend.
  • Just to show you 'another way' and not because I thought anything was wrong with your original choice, I replaced your use of a regular expression for finding the correct files with startsWith(...). Note there is also a method endsWith(...).
  • A bug could occur if the name of a file passed your filter predicate but is in fact a directory. I added a condition to to eliminate that possibility.

Cheers :)

Source Link
t. fochtman
  • 1.6k
  • 8
  • 5

Idiomatic Scala Style

  • Class, object, and trait names follow camelCase style with the first letter capitalized. One example where you don't follow this rule is abstract class spMeta extends logging.

  • For a few reasons, it is considered bad practice to user underscores in names. Applying the convention from the bullet point above with this we'd make the following transformation to your code: class sp_01 extends spMeta => class Sp01 extends SpMeta.

  • The above rules also extend to types that you define. Consequently I'd change type inputT to type InputT, etc.

  • Methods which act as accessors should be declared without parenthesis (unless they have side effects). In particular this applies to your def getSolution(): SolutionT = ... which should just be plain old parenthesis-less def getSolution: SolutionT = ...

  • If you ever have any style questions check out this documentation.

Code Suggestions

A modified version of your getSolutionFiles method with explanations below.

    def getSolutionFiles(dir: File): Array[File] = {
        val dirContents = dir.listFiles
        val targetFiles = dirContents filter(x => x.getName.startsWith("sp_") && !x.isDirectory)
        val childDirs   = dirContents filter(_.isDirectory)
        targetFiles ++ childDirs flatMap(d => getSolutionFiles(d))
    }
  • I renamed the method paramter to dir from file.
  • All of the operations in this method are on the container type Array. So I renamed some of the values to remove any mention of List.
  • Overall I think the renaming of these values make this method much easier to to immediately comprehend.
  • Just to show you 'another way' and not because I thought anything was wrong with your original choice, I replaced your use of a regular expression for finding the correct files with startsWith(...). Note there is also a method endsWith(...).
  • A bug could occur if the name of a file passed your filter predicate but is in fact a directory. I added a condition to to eliminate that possibility.

Cheers :)