current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

The following code has a lot of conditionals. I am trying to write it in a functional programming way.

val basePrice = {
  var b = 0.0

  if (runtime > 120)
    b += 1.5
  if ((day == Sat) || (day == Sun))
    b += 1.5
  if (!isParquet)
    b += 2
  if (is3D)
    b += 3
  b
}

I think the following code would be a good approach, but maybe I am complicating this too much.

val basePrice = {
  List((runtime > 120, 1.5), 
       (day == Sat || day == Sun, 1.5),
       (!isParquet, 2.0),
       (is3D, 3.0)).foldLeft(0.0)((acum, cond) =>
     if (cond._1) acum + cond._2 else acum)
}

How would you write the first snippet of code using functional programming?

Thanks

share|improve this question

4 Answers 4

up vote 5 down vote accepted

Each condition is a function. It might be that you could write it more concisely, but I think the code is clearer if you do this:

def priceFunction(cond: => Boolean)(mod: Double => Double) = (_: Double) match {
    case x if cond => mod(x)
    case y => y
}

val modRuntime = priceFunction(runtime > 120)(_ + 1.5)
val modWeekend = priceFunction(day == Sat || day == Sun)(_ + 1.5)
val modParquet = priceFunction(!isParquet)(_ + 2.0)
val mod3d = priceFunction(is3D)(_ + 3.0)
val modifiers = List(
    modRuntime,
    modWeekend,
    modParquet,
    mod3d
)
val modifierFunction = modifiers reduceLeft (_ andThen _)                   

val basePrice = modifierFunction(0.0)

The name of the identifiers here suck, and I could have written val modifiers = modRuntime andThen modWeekend andThen modParquet andThen mod3d without trouble. I choose putting them in a List because it shows how well it can scale.

One could also make PartialFunction and chain them with orElse, for the cases where you want only the first condition.

You see this kind of thing used in web frameworks, such as BlueEyes, Lift or Unfiltered, for example.

share|improve this answer
    
You've littered the code with one-shot vals, things which are unlikely to be directly re-used. Why not create a list of anonymous functions, at least? "modifiers = List( priceFunction(runtime > 120)(_ + 1.5), priceFunction(day == Sat || day == Sun)(_ + 1.5). etc, etc )". Frankly, I think simao's second example is as good as yours. –  itsbruce Aug 24 '13 at 17:15
    
@itsbruce For two reasons. First, the identifier name helps one understand what the function is doing. Second, functions are syntactically noisy, so it's difficult to see where one ends and another starts in a list, whereas the val declarations make that obvious. The main question is: why not declare them as val? It's not as if there would be any difference in memory usage or performance. –  Daniel C. Sobral Aug 25 '13 at 5:33
up vote 1 down vote

Maybe a little bit more readable:

val basePrice = List((runtime > 120, 1.5), 
       (day == Sat || day == Sun, 1.5),
       (!isParquet, 2.0),
       (is3D, 3.0)).collect{case (true, b) => b}.sum
share|improve this answer
    
Scala noob here. Would this cause the list to be traversed twice? One for collect and another for sum? Or scala is aware of that and chains it properly? –  simao Aug 17 '11 at 23:24
    
I think it would be traversed twice, but I really hope you don't want to chain hundreds of these... –  Landei Aug 18 '11 at 6:46
    
Note that you can use .view on lists to make them lazy (and, hence, traversed once). –  Nicolas Payette Sep 14 '11 at 5:02
up vote 0 down vote

I like Daniel C. Sobral's answer. Anyway, in addition to the List(...).sum alternatives, there's:

val basePrice = {
  0.0 +
    (runtime > 120 ? 1.5 | 0) +
    ((day == Sat) || (day == Sun) ? 1.5 | 0) +
    (!isParquet ? 2 | 0) +
    (is3D ? 3 | 0)
}

Where ?| is the ternary operator.

share|improve this answer
up vote 0 down vote

You can simply write it like this:

val basePrice = 0.0 +
  (if (runtime > 120) 1.5 else 0) +
  (if (day == Sat || day == Sun) 1.5 else 0) +
  (if (!isParquet) 2 else 0) +
  (if (is3D) 3 else 0)
share|improve this answer
    
This is not more functional, just a slightly different way of writing the same code. –  itsbruce Aug 24 '13 at 16:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.