CODEWITHSUNDEEP
smlsmlnj
guthik
guthik

Reputation: 404

What is wrong with my function

i have the following function that is supposed to return true if the passed argument is a reasonable date and false otherwise. the problem is that it is returning false even for obviously reasonable dates and i can't figure out what is wrong with it. anyone with sharper eyes please help. here it is: 

fun reasonable_date(x: int*int*int) =
    if #1 x > 0 andalso #2 x > 0 andalso #2 x <= 12 andalso #3 x > 0 andalso #3 x <= 31 
    then                                
    if #2 x = 1 mod 2 andalso #2 x < 8 andalso #3 x <= 31 then true
         else if #2 x = 0 mod 2 andalso #2 x >= 8 andalso #3 x <= 31 
         then true
     else if #2 x = 0 mod 2 andalso #2 x < 8
     then
         if #2 x = 2 andalso (#3 x = 28 orelse #3 x = 29) then true
         else if #2 x = 0 mod 2 andalso #3 x <= 30 then true
         else false
         else if #2 x = 1 mod 2 andalso #2 x > 8 andalso #3 x <=30 then true 
     else false
     else false

Upvotes: 0

Views: 257

Answers (3)

sonia
sonia

Reputation: 106

i prefer this solution which seems more readable

fun reasonable_date (y, m, d) =
    let val daysInMonth =
           List.nth([31, if isLeapYear y then 29 else 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31], m-1)
    in
        (* Check year >= 1 *)
        y >= 1 andalso

        (* Check 1 <= month <= 12 *)
        (m >= 1 andalso m <= 12) andalso

        (* Check 1 <= day <= n, for n being the number of days in specified month *)
        (d >= 1 andalso d <= daysInMonth)
    end

but may be i missed some tricks (i assume you already wrote an helper function isLeapYear)

Upvotes: -1

Jesper.Reenberg
Jesper.Reenberg

Reputation: 5944

Your current solution is impossible to maintain, and its logic looks like something that has been to hell and back :)

I would recommend that you break it up into smaller logical parts that ensure simple properties. Thus instead of first testing whether the year, month and day is greater or equal to one, you could group all the logic regarding years, months and days for itself

fun daysInMonth n =
    List.nth([31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31], n-1)

fun reasonable_date (y, m, d) =
    (* Check year >= 1 *)
    y >= 1 andalso

    (* Check 1 <= month <= 12 *)
    (m >= 1 andalso m <= 12) andalso

    (* Check 1 <= day <= n, for n being the number of days in specified month *)
    (d >= 1 andalso d <= daysInMonth m)

Obviously this doesn't handle leap years, however that is also quite simple to implement using a helper function, if the month is February. It could be done like this

fun reasonable_date (y, m, d) =
    (* Check year >= 1 *)
    y >= 1 andalso

    (* Check 1 <= month <= 12 *)
    (m >= 1 andalso m <= 12) andalso

    (* Check 1 <= day <= n, for n being the number of days in specified month *)
    (d >= 1 andalso
     (* If February, and leap year *)
     ((m = 2 andalso isLeapYear y andalso d <= 29)
      (* Any other month or non leap year *)
      orelse d <= daysInMonth m))

Upvotes: 2

Michael J. Barber
Michael J. Barber

Reputation: 25032

You repeatedly use conditions like if #2 x = 1 mod 2. This is almost certainly does not work as you think it does. Here, mod is an arithmetic operator, meaning the remainder obtained when dividing 1 by 2, and not the mathematical expression saying that #2 x equals 1 modulo 2. Thus, instead of testing whether #2 x is odd, you're testing whether it equals 1. Following through your conditions, you really only allow true when #2 x is 1, so your reasonable dates must all be in January (and there may not even be any, I haven't worked through all conditions).

Upvotes: 1

Related Questions