8

I have been trying to write a program in F# which takes two values (a tuple): a number and a string. Depending on whether the string tells the program to add or multiply, it will add or multiply the input number with all the integers from 1 to that number (i.e. 1..n).

Here is the code

let addormult (n:int, what:string) = 
    if what = "multiply" then 
       let rec product n = 
           if n<1 then 0
           else n*product(n-1)
       printfn "%A" product
    elif what = "sum" then
         let rec sum = 
             if n<1 then 0
             else n + sum(n-1)
         printfn "%A" sum

However, every time I try to run this function, I get the error

"This value is not a function and cannot be applied."

So what am I doing wrong?

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
Clement Decker
  • 137
  • 1
  • 6
  • 1
    I guess, you forgot to call the function – FoggyFinder Aug 10 '16 at 19:50
  • Instead of `product(n-1)`, try `product n-1` (though I am not sure about the order of operations here). I think the parenthesis are trying to turn it into a 1-value tuple. – Max Aug 10 '16 at 19:53
  • in addition, you do not consider the option when `what <> "mul" && what <> "add"` – FoggyFinder Aug 10 '16 at 19:53
  • if fix it, you got: https://dotnetfiddle.net/yOaITi – FoggyFinder Aug 10 '16 at 19:55
  • but you will still have an incorrect result when multiplying. You need to return `1`, I suppose. – FoggyFinder Aug 10 '16 at 19:56
  • @FoggyFinder I have called the function. Also, can you explain what we mean in your last comment – Clement Decker Aug 10 '16 at 20:20
  • @Max already gave the answer. Now you realized what your mistake was? If not, I'll try to explain. – FoggyFinder Aug 10 '16 at 20:25
  • 1
    Since you are just starting to learn F#, I would advise you to join [F# Software Foundation](http://foundation.fsharp.org/join) and when you have 20 reputation points, you will be able to join [SO chat](http://chat.stackoverflow.com/rooms/51909/f) although it is not as active as F#-slack. – FoggyFinder Aug 10 '16 at 20:31
  • Note the recursive functions are not _tail recursive_ so can result in stack overflow for large enough N. Better to do with a regular mutable variable and an imperative loop. See http://stackoverflow.com/a/37010/171121 for an example of how to do it tail-recursively (in python). – Dax Fohl Aug 11 '16 at 00:50
  • 1
    @ClementDecker If one of the answers helped you resolve the issue, please remember to select it. :) – Max Aug 11 '16 at 18:26

3 Answers3

8

Really more of an extended comment, but the whole of this code can be simplified by using a fold function.

let addormult (n, what) =
  let f = if what = "multiply" then (*) else (+)
  List.fold f 1 [1..n]
let x = addormult(4, "multiply") // results in x = 24
let y = addormult(4, "add")      // results in y = 10

Or even better, define getOp outside the scope, since it's going to be generally applicable.

let getOp what = if what = "multiply" then (*) else (+)
let addormult (n, what) = List.fold (getOp what) 1 [1..n]
let x = addormult(4, "multiply") // results in x = 24
let y = addormult(4, "add")      // results in y = 10

fold is also tail-recursive, ensuring you don't exceed the stack size limit for large N. In F#, a lot of times when you do recursion, there's already a standard library function that does what you need. Or if not, sometimes it's better to extract a "general" recursion out of your function and then implement your specific thing in terms of that.

Now, note that strings are not the best way to convey intention here. Far better to use a discriminated union to convey your operation:

type Op = Add | Multiply
let getop = function | Multiply -> (*) | Add -> (+)
let addormult (n, what) = List.fold (getop what) 1 [1..n]
let x = addormult(4, Multiply) // results in x = 24
let y = addormult(4, Add)      // results in y = 10

That way there's no chance someone could accidentally type "mutliply" and get an unexpected return value.

But really there's no reason to limit to just two operations. It's straightforward to make this work with any operation:

let doOneToN f n = List.fold f 1 [1..n]
let x0 = doOneToN (*) 4 // results in x0 = 24
let y0 = doOneToN (+) 4 // results in y0 = 10

With this, you can easily use partial application to make specialized functions if you want:

let factorial = doOneToN (*)     // the `n` argument is "curried"
let triangularSum = doOneToN (+) // the `n` argument is "curried"
let x1 = factorial 4             // results in x1 = 24
let y1 = triangularSum 4         // results in y1 = 10

That's the nice thing about functional programming, is that it's easy to mix and match things.

Of course it's so simple that you might not even care to make a function. It's almost simpler just to call let x = List.fold (*) 1 [1..4] inline. Functional-first languages tend to be nice and terse that way.

Dax Fohl
  • 10,654
  • 6
  • 46
  • 90
7

Prefix: I am no F# expert and someone more knowledgeable than me may be able to provide a more complete answer.

There are a couple of things to fix here:

  1. let rec sum = is missing a parameter variable declaration.
  2. printfn "%A" product (and the one for sum) are printing the functions, not the values. To correct this, you need to call the function with the arguments.
  3. Defining n for both the addormult tuple value and the recursive value might cause issues - though I am not sure how F# deals with scoping in a situation like this.

This is the "multiply" version after these changes:

let addormult (n:int, what:string) = 
if what = "multiply" then 
   let rec product x = 
       if x = 1 then 1
       else x * product(x-1)       
   printfn "product is %A" (product n)

And here is the call:

let x = addormult (4, "multiply")

Which gives me this value:

product is 24
Max
  • 849
  • 9
  • 24
7

In this part of the code:

let rec sum = 
    if n<1 then 0
    else n + sum(n-1)

sum is defined as a value because it takes no arguments. The third line attempts to call it as a function (sum(n-1)), which isn't possible because it isn't a function.

Add an argument to the function, e.g.

let rec sum x = 
    if x<1 then 0
    else x + sum(x-1)

Here I took the liberty and replaced n in that function body with x, although I don't know if that's what you want to do. It compiles, though.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • 1
    I applied the suggestions, and I am getting the right value now. However, for some reason F# interactive is telling me that my value is a unit, even though I am obtaining a value as my result. – Clement Decker Aug 10 '16 at 20:33
  • 1
    @ClementDecker Yes, the type of the `addormult` function is `n:int * what:string -> unit`. It's printing to the console, which returns `unit`, and those expressions are the last expression in each branch. So that becomes the function's return type. – Mark Seemann Aug 10 '16 at 20:43
  • 1
    @Clement Decker, you print to console the value instead of return it from the function. So F# interactive says that the type unit. – FoggyFinder Aug 10 '16 at 20:43
  • @ClementDecker If you want to return the formatted string instead of printing it to the console, use `sprintf`. – Mark Seemann Aug 10 '16 at 20:44
  • Thank you! The OP asked "what am I doing wrong?" not "please refactor my code...", and your answer is what helped/helps novice like me – HidekiAI Jan 21 '20 at 18:23