Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Done with all the task #3

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/main/scala/DataMunging.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package DataMunging
import scala.io.Source

object DataMunging {

def main(args: Array[String]) ={
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove def main(args: Array[String]) by just extending App, that is, object DataMunging extends App

getMinSpread()
getMinGoalDiff()
}
def getMinSpread() = {
val source = Source.fromFile("weather.dat")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also factor out

  1. the code to fetch all the lines in the function, because that is common to both getMinSpread() and getMinGoalDiff().
  2. filter out the header and the lines with "-----" and the blank line in this function.
  3. and you tokenise (split) by space.
    That is:
def getLines(val fileName:String):List[Array[String]] = {
               val source = Source.fromFile(fileName)
               val lines = source.getLines.toList
                source.close
               lines.drop(0)
                       .filter(line=> !line.contains("-----"))
                       .filter(line => !line.trim.isEmpty)
                       .map(_.split("\\s+"))
   }

val spread = source.getLines.toList
.drop(2) //the first line is a header and second line is a blank space so skip those lines here.
.map(_.split("\\s+")) // split on regex white space
.map(x => (x(1), x(2).stripSuffix("*").toDouble-x(3).stripSuffix("*").toDouble)) //clean the recourds as somelines have * and aslo conver to double
val min_spread = find_min(spread) // get minimum spread
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scala style guide usually goes with camelCase variable names like minSpread, rather than having underscores like min_spread

http://docs.scala-lang.org/style/

println(s"1: the day with the smallest temparature spread is day ${min_spread._1} with a spread of ${min_spread._2} degrees")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

println at this point is okay. However your getMinSpread() function can be more testable with a unit test if you make it return the actual spread. In other words, making it 'a pure function without side-effects' .
The println can be cone in the main method. So your main method would look like:

def main(args: Array[String]) ={
    val minSpread = getLines("weather.dat"). getMinSpread
    println(s"1: the day with the smallest temparature spread is day ${minSpread._1} with a spread of ${minSpread._2} degrees")
    val team = getLines("football.dat").getMinGoalDiff()
    println(s"2: the team with the smallest difference for and against goal is ${team._1} with a difference of ${team._2} degrees")
 }

source.close()
}

def getMinGoalDiff() = {
val source = Source.fromFile("football.dat") //get file access object
val teams = source.getLines.toList
.drop(1) //drop the header
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you implement getLines the way I suggest, getMinGoalDiff() becomes
def getMinGoalDiff(line: Array[String):(String, Double).
Also lines 22 to 26 will no longer be needed.

.filter(x=> !x.endsWith("-")) // filter out line with ----
.map(_.split("\\s+")) // split each line on white space
.map(x =>(x(2), x(7).toDouble - x(9).toDouble)) // select team and different between f and a
val team = find_min(teams)
println(s"2: the team with the smallest difference for and against goal is ${team._1} with a difference of ${team._2} degrees")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice touch with the string interpolation 👍

source.close()
}
//
// def getSourceData(filename: String): BufferedSource = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can delete commented out code to keep your code clean

// return Source.fromFile(filename)
// }

def find_min(data: List[(String, Double)]): (String, Double)= {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job factoring out this common code 👍

return data.reduceLeft((x,y) => if (x._2 < y._2) x else y)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, very very good effort 👍 👍 . For someone new to Scala, you have done a great job!