Search code examples
smlsmlnjml

Show Multiple occurrences of items in a list, sml


I'm trying to show the indices of all multiple occurrences for an element in a list.

in standard ml.

but my code show the empty list :(

this is my code:

    fun index(item, xs,ys) =
let
fun index'(m, nil , ys) = (
  SOME (ys) )
| index'(m, x::xr , ys) = if x = item then(
  (ys @ [m]);
  index'(m + 1, xr , ys)
  )
  else index'(m + 1, xr,ys)
in
index'(0, xs , ys)
end;


index(1,[1,2,1,4,5,1],[]);

can you help me?


Solution

  • First things first:

    1. index should take two arguments; the element to look for and the list to look for it in.
      The accumulation parameter belongs to the helper function.
    2. It is meaningless to always produce SOME of something and never NONE.

    Let's fix these first.

    fun index (item, xs) =
        let
            fun index'(m, nil , ys) = ys
              | index'(m, x::xr, ys) = if x = item then(
                                           (ys @ [m]);
                                           index'(m + 1, xr , ys)
                                       )
                                       else index'(m + 1, xr,ys)
        in
            index'(0, xs, [])
    end;
    

    Now you don't need to pass the extra accumulator parameter when you use index.
    It's also impossible to start with something other than [].

    Your next, and main, problem is

    (ys @ [m]);
    index'(m + 1, xr , ys)
    

    which first creates the list ys @ [m], immediately throws it away, and then produces as its result index'(m + 1, xr , ys), which is exactly what the else branch does.

    That is, the conditional is equivalent to

    if x = item
    then index'(m + 1, xr, ys)
    else index'(m + 1, xr, ys)
    

    and thus, index' is equivalent to

    fun index'(m, nil, ys) = ys
      | index'(m, x::xr, ys) = index'(m + 1, xr, ys)
    

    Since you always pass the original ys along, and it is [] to start with, the result is always [].

    What you need to do is to pass the extended list to the recursion, so it can become the result when the recursion terminates.
    Renaming the accumulator ys to make its purpose clearer:

    fun index (item, xs) =
        let
            fun index'(i, nil, accumulator) = accumulator
              | index'(i, x::xr, accumulator) = if x = item
                                                then index' (i + 1, xr, accumulator @ [i])
                                                else index' (i + 1, xr, accumulator)
        in
            index'(0, xs, [])
    end;
    

    This is inefficient, due to the repeated appending of an element to the back of a list.
    It is very common to accumulate in reverse, and correct it when you're done.
    (This "feels" inefficient, but it isn't.)

    fun index (item, xs) =
        let
            fun index'(i, nil, accumulator) = List.reverse accumulator
              | index'(i, x::xr, accumulator) = if x = item
                                                then index' (i + 1, xr, i::accumulator)
                                                else index' (i + 1, xr, accumulator)
        in
            index'(0, xs, [])
    end;