number_in_month exercise (Count elements in a list)

Question

I have been trying to count elements in a list of integer 3-tuples, that equals a given integer using SML, but it's not working. Can anyone help me figure out what's wrong with the below code or straighten it up for me?

 fun number_in_month(x : int*int*int list, m: int) =
    if null x then 0

         else
           let fun inc x = x + 1;
         in
             val counter = 0;
             if m = #2 (hd x) andalso m > 0 then inc counter
            number_in_month((tl x), m)
           `  else
             number_in_month((tl x), m)
        end

This function is supposed to return the number of times m equals to the second element of each tuple in the list.

Answer 1

Clearly you have a hard time to let go of your imperative thinking.

Let me try and address some of your issues

• You should be using pattern matching instead of using null x, hd x and tl x. This also apply to decomposing tuples and records. For example

  fun number_in_month ((x1, x2, x3) :: xs, m) = ...
  

  or, since we don't ever use x1 and x3

  fun number_in_month ((_, x2, _) :: xs, m) = ...
  

  This way it is clearly seen that the first argument is a list of 3-tuples, and no type annotation is needed

  Also when you omit the explicit type annotation, which is the whole idea of having a type system that can infer them for you (see next point), then this code

  fun foo42 xs = map (fn x => #2 x) xs
  

  will give you some nasty errors on "unresolved flex record" (this error message is from SML/NJ)

  /tmp/sml20620PlF:105.5-105.44 Error: unresolved flex record
     (can't tell what fields there are besides #2)
  

  which is easily fixed by decomposing the 3-tuple

  fun foo42 xs = map (fn (_, x2, _) => x2) xs
  

• Speaking of type annotations. They are (almost always) not needed, and they clutter up the readability of the code. Not to mention that they unnecessarily restricts the types you function may be used on.

  Also the type annotation you have given is erroneous according to what you really wan't. You should have places parenthesis around the int * int * int. Currently it is interpreted as a 3-tuple of two ints and an int list int * int * (int list).

  If you really insist in type annotating your function, then you can do it like this

  val number_in_month : (int * int * int) list * int -> int = 
      fn ([]            , m) => 0
       | ((_,x2,_) :: xs, m) => 42
  

  This is "almost" like Haskell, where the type is given just before the function declaration.

• Try to be more consistent in they way you indent your code. That will give you better clarity. Here I'm specifically thinking of the way you have indented the else part end the in ... end part. The below part is clearly still erroneous in so many ways i can't begin to imagine, but it gives an idea as how to do it

  fun number_in_month(x : int*int*int list, m: int) =
      if null x then 0
      else
        let fun inc x = x + 1;
        in
          val counter = 0;
          if m = #2 (hd x) andalso m > 0 then
             inc counter
             number_in_month((tl x), m)
          else
             number_in_month((tl x), m)
        end
  

• You can't declare a variable val counter = 0 inside the in ... end part of a let-expression. The semantics of a let-expression is

  let
    dec
  in
    exp_1; ...; exp_n
  end
  

  thus all declarations (function and value bindings, etc) must go in the let ... in part.

• There is no need on earth to have an increment function, it just clutters the readability. Remember that SML uses single assignment, thus variables are immutable after they are declared.

• The sequence-thing inside your nested if-expression

  inc counter
  number_in_month((tl x), m)
  

  makes absolutely no sense. The only way you can have more than one expression inside the then ... else part (actually any place, where a single expression is expected), is with a sequence (exp_1; ...; exp_n). However this is only usable when all but the last expression has side effect(s), as their results is ignored/thrown away

  - (print "Foo\n"; print "Bar\n"; 42);
  Foo
  Bar
  val it = 42 : int
  

If you search a bit here on SO, you will see that a quite similar question has recently been asked and answered. Though it differs in the the type of the last argument, you might still get some useful pointers.

All in all a solution might look like

fun number_in_month ([], _) = 0
  | number_in_month ((_,x2,_) :: xs, m) = 
    if x2 = m then
      1 + number_in_month(xs, m)
    else
      number_in_month(xs, m)

However since your problem is simpler than the previously stated one, you could easily use some of the higher-order functions from the list module in the basis library

fun number_in_month (xs, m) = length (List.filter (fn (_, x2, _) => x2 = m) xs)

Or even (arguably) simpler, by folding over the list and incrementing a variable along the way each time it matches

fun number_in_month (xs, m) = foldl (fn ((_, x2, _), b) => if x2 = m then b+1 else b) 0 xs

Answer 2

fun number_in_month (L : (int*int*int) list, m : int) =
if L = nil
then 0
else
    (if #2 (hd L) = m then 1 else 0) + number_in_month (tl L,m);

TESTING:

number_in_month ([] , 2);
number_in_month ([(1,2,3)] , 2);
number_in_month ([(1,2,3),(2,2,2)] , 2);
number_in_month ([(1,2,3),(2,2,2),(19,11,29)] , 2);
number_in_month ([(1,2,3),(2,2,2),(19,11,29),(10,28,19)] , 2);
number_in_month ([(1,2,3),(2,2,2),(19,11,29),(10,2,19)] , 2);
number_in_month ([(1,2,3),(2,2,2),(19,11,29),(10,28,19)] , 2);
number_in_month ([(1,2,3),(2,2,2),(19,11,29),(10,28,19)] , 2);
number_in_month ([(1,2,3),(2,2,2),(19,11,29),(10,28,19),(16,2,7)] , 2);

Reference:

http://www.cs.sunysb.edu/~leo/CSE215/smllistexamples.txt

http://www.standardml.org/Basis/list.html