CODEWITHSUNDEEP
f#
verticese
verticese

Reputation: 273

Assignment: this expression was expected to have the type unit

I'm writing some thing really easy, a program that finds all factors of an int. Here is what I have 

let factor n=
  let ls=[]
  for i=1 to n do
    if i % n =0 then ls = i::ls
  l

If I do this then it pops an error This expression was expected to have the type unit. But I tried to put an expression that prints something after if..then.., which is suppose to return type unit, but it still gives the same error. I am lost about this. Can someone help please? Thanks

Upvotes: 0

Views: 124

Answers (2)

ildjarn
ildjarn

Reputation: 62975

= is comparison, not assignment. You want either

let factor n =
    let mutable ls = []
    for i = 1 to n do
        if n % i = 0 then ls <- i::ls
    ls

or

let factor n =
    let ls = ref []
    for i = 1 to n do
        if n % i = 0 then ls := i::(!ls)
    !ls

Note, however, that both of these solutions are highly unidiomatic, as there are equally easy immutable solutions to this problem.

Upvotes: 1

Mau
Mau

Reputation: 14468

You are trying to make ls into a mutable variable nd assign it with =. While this is possible, by using mutable (1) or ref (2) along with <- or := assignment operators, this is generally discouraged in the functional world.
A possibly more idiomatic implementation of the naive algorithm could be:

let factor n =
    let rec factorLoop curr divs =
        if curr > n then divs
        else
            if n % curr = 0
            then factorLoop (curr+1) (curr::divs)
            else factorLoop (curr+1) divs
    factorLoop 1 [] |> List.rev

> factor 12;;
val it : int list = [1; 2; 3; 4; 6; 12]

Here the main function defines an inner factorLoop function that is recursive. Recursion is the way we can avoid many uses of mutable variables in functional languages. The recursive inner function threads along a curr variable that is the current divisor to be tested and a list divs of currently found divisors. The result includes 1 and n. This can be altered respectively by changing the initial value of curr and the terminating condition in the first line of factorLoop.

It is worth noting that it can all be shrunk down to one line by making use of the F# library:

let factor n =
    [1..n] |> List.filter (fun x -> n % x = 0)

Here we build a list of values 1..n and feed them to List.filter which applies the given predicate (at the end of the line) to select only divisors on n. If n is large, however, the temp list will grow very large. We can use a lazily evaluated sequence instead, which won't blow the memory usage:

let factor n =
    {1..n} |> Seq.filter (fun x -> n % x = 0) |> Seq.toList

Here we filter on a 'lazy' sequence and only convert the (much smaller) sequence of results to a list at the end:

> factor 10000000;;
val it : int list =
  [1; 2; 4; 5; 8; 10; 16; 20; 25; 32; ... etc

Upvotes: 2

Related Questions