tags:

views:

115

answers:

2

My first Scala program and I am stuck.

So basically i am trying to override the potential None value of "last" to a 0l in the declaration of past.

import java.util.Date;

object TimeUtil {
    var timerM = Map( "" -> new Date().getTime() );

    def timeit(seq:String, comment:String) {
        val last = timerM.get(seq) 
        val cur = new Date().getTime()
        timerM += seq -> cur;
        println( timerM )
        if( last == None ) return;

        val past = ( last == None ) ? 0l : last  ;
        Console.println("Time:" + seq + comment + ":" + (cur - past)/1000  )
    }

    def main(args : Array[String]) {
        timeit("setup ", "mmm")
        timeit("setup ", "done")
    }
}
+7  A: 

You should probably try and distil the problem a little more, but certainly include your actual error!

However, there is no ternary operator (? :) in Scala so instead you can use:

if (pred) a else b //instead of pred ? a : b

This is because (almost - see Kevin's comments below) everything in scala is an expression with a return type. This is not true in Java where some things are statements (with no type). With scala, it's always better to use composition, rather than forking (i.e. the if (expr) return; in your example). So I would re-write this as:

val last = timerM.get(seq)
val cur = System.currentTimeMillis
timerM += (seq -> cur)
println(timerM)
last.foreach{l => println("Time: " + seq + comment + ":" + (l - past)/1000 ) }  

Note that the ternary operation in your example is extraneous anyway, because last cannot be None at that point (you have just returned if it were). My advice is to use explicit types for a short while, as you get used to Scala. So the above would then be:

val last: Option[Long] = timerM.get(seq)
val cur: Long = System.currentTimeMillis
timerM += (seq -> cur)
println(timerM)
last.foreach{l : Long => println("Time: " + seq + comment + ":" + (l - past)/1000 ) }  

(It seems in your comment that you might be trying to assign a Long value to last, which would be an error of course, because last is of type Option[Long], not Long)

oxbow_lakes
thanks. suppose this works too Console.println("Hi:"+ ( cur - last.getOrElse(0L) )
smartnut007
I, too, made this mistake. Not everything in Scala is an expression, variable, class and method definitions are statements, as are import statements and (I think) assignment. When the compiler encounters a code block that terminates with a statement, it'll insert the expression `()` at the end if you try to evaluate that block - thus giving the *appearance* that everything is an expression.
Kevin Wright
You're wrong about assignment; it is an expression of type `Unit` but the point still stands. I should have been clearer
oxbow_lakes
Yeah, I wasn't 100% sure on that one myself.
Kevin Wright
+4  A: 

You have a couple of "code smells" in there, which suggest that a better, shinier design is probably just around the corner:

  • You're using a mutable variable, var instead of val
  • timeit works exclusively by side effects, it modifies state outside of the function and successive calls with the same input can have different results.
  • seq is slightly risky as a variable name, it's far too close to the (very common & popular) Seq type from the standard library.

So going back to first principles, how else might the same results be achieved in a more "idiomatic" style? Starting with the original design (as I understand it):

  • the first call to timeit(seq,comment) just notes the current time
  • subsequent calls with the same value of seq println the time elapsed since the previous call

Basically, you just want to time how long a block of code takes to run. If there was a way to pass a "block of code" to a function, then maybe, just maybe... Fortunately, scala can do this, just use a by-name param:

def timeit(block: => Unit) : Long = {
  val start = System.currentTimeMillis
  block
  System.currentTimeMillis - start
}

Just check out that block parameter, it looks a bit like a function with no arguments, that's how by-name params are written. The last expression of the function System.currentTimeMillis - start is used as the return value.

By wrapping the parameter list in {} braces instead of () parentheses, you can make it look like a built-in control structure, and use it like this:

val duration = timeit {
  do stuff here
  more stuff
  do other stuff
}

println("setup time:" + duration + "ms")

Alternatively, you can push the println behaviour back into the timeit function, but that makes life harder if you later want to reuse it for timing something without printing it to the console:

def timeit(description: String)(block: => Unit) : Unit = {
  val start = System.currentTimeMillis
  block
  val duration System.currentTimeMillis - start
  println(description + " took " + duration + "ms")
}

This is another trick, multiple parameter blocks. It allows you to use parentheses for the first block, and braces for the second:

timeit("setup") {
  do stuff here
  more stuff
  do other stuff
}
// will print "setup took XXXms"

Of course, there's a million other variants you can make on this pattern, with varying degrees of sophistication/complexity, but it should be enough to get you started...

Kevin Wright
I have a heavy project and wanted to introduce some scala code as a learning experience.
smartnut007
But, not sure how to call this from java.
smartnut007
while( ( line = br.readLine() ) != null ){ if( line.length() < 140 ) continue; addDoc(w, line); if( ++N % 100000 == 0) TimeUtil.timeit("batch ", ""+ N/1000 ); }
smartnut007
Ah, I think I will have to define Function0<Long> in java and use it. But, not very pretty.
smartnut007
Oh no, this is pure Scala code - I wouldn't use it if Java interop was a requirement. That's probably the sort of thing worth mentioning in your original question!
Kevin Wright
@smartnut007 Defining a `Function0<Long>` isn't all that different from defining a `Runnable`, a `Comparator`, or any other of the one-method classes from Java, is it?
Daniel
@Daniel No, not really
Kevin Wright
@Kevin it was a rhetorical question... ;-)
Daniel
@Daniel that's okay, it was a rhetorical answer...
Kevin Wright
@kevin lol! :-)
Daniel