There is definitely an error but for the life of me I can't see it

[Jaze]

I'm trying to test whether a given string is in the format of a call time. Call times are either H:MM:SS or MM:SS or M:SS
I keep getting a result of TRUE for "Cheese" and other seemingly random strings. I just don't know what the error is

Code: QB64: [Select]

1. CONST TRUE = 1
2. CONST FALSE = -1
3.  
4. FUNCTION IsCallTime (possibleCallTimeString$)
5.   rtn = TRUE
6.   pCt$ = possibleCallTimeString$
7.   IF LEN(pCt$) = 4 THEN
8.     '1234
9.     'M:SS
10.     IF MID$(pCt$, 2, 1) <> ":" THEN rtn = FALSE
11.     n1$ = LEFT$(pCt$, 1)
12.     IF ASC(n1$) < 48 OR ASC(n1$) > 57 THEN rtn = FALSE
13.     n2$ = MID$(pCt$, 3, 1)
14.     IF ASC(n2$) < 48 OR ASC(n2$) > 57 THEN rtn = FALSE
15.     n3$ = MID$(pCt$, 4, 1)
16.     IF ASC(n3$) < 48 OR ASC(n3$) > 57 THEN rtn = FALSE
17.     IF VAL(n1$ + n2$ + n3$) = 0 THEN rtn = FALSE
18.   ELSEIF LEN(pCt$) = 5 THEN
19.     '12345
20.     'MM:SS
21.     PRINT
22.     IF MID$(pCt$, 3, 1) <> ":" THEN rtn = FALSE
23.     n1$ = MID$(pCt$, 1, 1)
24.     IF ASC(n1$) < 48 OR ASC(n1$) > 57 THEN rtn = FALSE
25.     n2$ = MID$(pCt$, 2, 1)
26.     IF ASC(n2$) < 48 OR ASC(n2$) > 57 THEN rtn = FALSE
27.     n3$ = MID$(pCt$, 4, 1)
28.     IF ASC(n3$) < 48 OR ASC(n3$) > 57 THEN rtn = FALSE
29.     n4$ = MID$(pCt$, 5, 1)
30.     IF ASC(n4$) < 48 OR ASC(n4$) > 57 THEN rtn = FALSE
31.     IF VAL(n1$ + n2$ + n3$ + n$4) = 0 THEN rtn = FALSE
32.   ELSEIF LEN(pCt$) = 7 THEN
33.     '1234567
34.     'H:MM:SS
35.     IF MID$(pCt$, 2, 1) <> ":" THEN rtn = FALSE
36.     IF MID$(pCt$, 5, 1) <> ":" THEN rtn = FALSE
37.     n1$ = MID$(pCt$, 1, 1)
38.     IF ASC(n1$) < 48 OR ASC(n1$) > 57 THEN rtn = FALSE
39.     n2$ = MID$(pCt$, 3, 1)
40.     IF ASC(n2$) < 48 OR ASC(n2$) > 57 THEN rtn = FALSE
41.     n3$ = MID$(pCt$, 4, 1)
42.     IF ASC(n3$) < 48 OR ASC(n3$) > 57 THEN rtn = FALSE
43.     n4$ = MID$(pCt$, 6, 1)
44.     IF ASC(n4$) < 48 OR ASC(n4$) > 57 THEN rtn = FALSE
45.     n5$ = MID$(pCt$, 7, 1)
46.     IF ASC(n5$) < 48 OR ASC(n5$) > 57 THEN rtn = FALSE
47.     IF VAL(n1$ + n2$ + n3$ + n4$ + n5$) = 0 THEN rtn = FALSE
48.   END IF
49.   IsCallTime = rtn
50. END FUNCTION

[bplus]

You might have more luck with False = 0 and True = -1

Don't even set the rtn until you know it's True.


Does this work for you?

Code: QB64: [Select]

1. Const TRUE = -1
2. Const FALSE = 0
3.  
4. Do
5.     Input "Test an call hour: "; test$
6.     Print IsCallTime(test$)
7.     Print
8. Loop Until test$ = ""
9.  
10. Function IsCallTime (possibleCallTimeString$)
11.  
12.     pCt$ = possibleCallTimeString$
13.     If Len(pCt$) = 4 Then
14.         '1234
15.         'M:SS
16.         If Mid$(pCt$, 2, 1) = ":" Then
17.             n1$ = Left$(pCt$, 1)
18.             If Asc(n1$) >= 48 And Asc(n1$) <= 57 Then
19.                 n2$ = Mid$(pCt$, 3, 1)
20.                 If Asc(n2$) >= 48 And Asc(n2$) <= 57 Then
21.                     n3$ = Mid$(pCt$, 4, 1)
22.                     If Asc(n3$) >= 48 And Asc(n3$) <= 57 Then
23.                         If Val(n1$ + n2$ + n3$) <> 0 Then rtn = TRUE
24.                     End If
25.                 End If
26.             End If
27.         End If
28.     ElseIf Len(pCt$) = 5 Then
29.         '12345
30.         'MM:SS
31.         Print
32.         If Mid$(pCt$, 3, 1) = ":" Then
33.             n1$ = Mid$(pCt$, 1, 1)
34.             If Asc(n1$) >= 48 And Asc(n1$) <= 57 Then
35.                 n2$ = Mid$(pCt$, 2, 1)
36.                 If Asc(n2$) >= 48 And Asc(n2$) <= 57 Then
37.                     n3$ = Mid$(pCt$, 4, 1)
38.                     If Asc(n3$) >= 48 And Asc(n3$) <= 57 Then
39.                         n4$ = Mid$(pCt$, 5, 1)
40.                         If Asc(n4$) >= 48 And Asc(n4$) <= 57 Then
41.                             If Val(n1$ + n2$ + n3$ + n$4) <> 0 Then rtn = TRUE
42.                         End If
43.                     End If
44.                 End If
45.             End If
46.         End If
47.  
48.     ElseIf Len(pCt$) = 7 Then
49.         '1234567
50.         'H:MM:SS
51.         If Mid$(pCt$, 2, 1) = ":" Then  ' edit fix this too!
52.             If Mid$(pCt$, 5, 1) = ":" Then  'edit fix this too!
53.                 n1$ = Mid$(pCt$, 1, 1)
54.                 If Asc(n1$) >= 48 And Asc(n1$) <= 57 Then
55.                     n2$ = Mid$(pCt$, 3, 1)
56.                     If Asc(n2$) >= 48 And Asc(n2$) <= 57 Then
57.                         n3$ = Mid$(pCt$, 4, 1)
58.                         If Asc(n3$) >= 48 And Asc(n3$) <= 57 Then
59.                             n4$ = Mid$(pCt$, 6, 1)
60.                             If Asc(n4$) >= 48 And Asc(n4$) <= 57 Then
61.                                 n5$ = Mid$(pCt$, 7, 1)
62.                                 If Asc(n5$) >= 48 And Asc(n5$) <= 57 Then
63.                                     If Val(n1$ + n2$ + n3$ + n4$ + n5$) <> 0 Then rtn = TRUE
64.                                 End If
65.                             End If
66.                         End If
67.                     End If
68.                 End If
69.             End If
70.         End If
71.     End If
72.     IsCallTime = rtn
73. End Function
74.  
75.  

OK I didn't finish flipping < to >= and > to <= in the last case and switch ing Or with and like I did in other 2 cases. OK I think I have em all now?

I don't think you need the add values test.

[SMcNeill]

The reason is really very simple -- "cheese" is 6 letters.  Your sub only checks to invalidate 4, 5, and 7 letter words that aren't in your associated time format.

You start out assuming the format is TRUE, and then never prove that it's not with anything outside of those 3 possible combinations.

You probably need a:

ELSE
    rtn = FALSE

before the END IF.

For ease of emphasis look at this: 

Code: QB64: [Select]

1. Function IsCallTime (possibleCallTimeString$)
2.     rtn = TRUE
3.     pCt$ = possibleCallTimeString$
4.     If Len(pCt$) = 4 Then
5.     ElseIf Len(pCt$) = 5 Then
6.     ElseIf Len(pCt$) = 7 Then
7.     End If
8.     IsCallTime = rtn
9. End Function

If length is 4, or 5, or 7.... then...

"cheese" is a length 6.  It's not going to be in any of those IF statements, so it's going to stay TRUE as you set it beforehand.

[bplus]

But what if someone does this 1:72? It would pass tests but be improper m:ss.

[SMcNeill]

But what if someone does this 1:72? It would pass tests but be improper m:ss.

Try it.  It comes back false.

        n4$ = Mid$(pCt$, 5, 1)
        If Asc(n4$) < 48 Or Asc(n4$) > 57 Then rtn = FALSE

Your 5th character isn't a number from 0 to 9...  It's FALSE.

[luke]

You will have a much easier time of things if you start by assuming the input does not match, and only setting the return value to true when it does.

[bplus]

Try it.  It comes back false.

        n4$ = Mid$(pCt$, 5, 1)
        If Asc(n4$) < 48 Or Asc(n4$) > 57 Then rtn = FALSE

Your 5th character isn't a number from 0 to 9...  It's FALSE.

I did try it and it came back true, I am testing my code fix. Need to add test for val(secs) <= 59 and val(mins) <= 59 or test the tens spot for mins and secs < asc("6"), the later is same amount of checks so go with that.

[bplus]

Here is new fix:

Code: QB64: [Select]

1. Const TRUE = -1
2. Const FALSE = 0
3.  
4. Do
5.     Input "Test an call hour: "; test$
6.     Print IsCallTime(test$)
7.     Print
8. Loop Until test$ = ""
9.  
10. Function IsCallTime (possibleCallTimeString$)
11.  
12.     pCt$ = possibleCallTimeString$
13.     If Len(pCt$) = 4 Then
14.         '1234
15.         'M:SS
16.         If Mid$(pCt$, 2, 1) = ":" Then
17.             n1$ = Left$(pCt$, 1)
18.             If Asc(n1$) >= 48 And Asc(n1$) <= 57 Then
19.                 n2$ = Mid$(pCt$, 3, 1)
20.                 If Asc(n2$) >= 48 And Asc(n2$) <= Asc("6") Then
21.                     n3$ = Mid$(pCt$, 4, 1)
22.                     If Asc(n3$) >= 48 And Asc(n3$) <= 57 Then
23.                         If Val(n1$ + n2$ + n3$) <> 0 Then rtn = TRUE
24.                     End If
25.                 End If
26.             End If
27.         End If
28.     ElseIf Len(pCt$) = 5 Then
29.         '12345
30.         'MM:SS
31.         Print
32.         If Mid$(pCt$, 3, 1) = ":" Then
33.             n1$ = Mid$(pCt$, 1, 1)
34.             If Asc(n1$) >= 48 And Asc(n1$) <= Asc("6") Then
35.                 n2$ = Mid$(pCt$, 2, 1)
36.                 If Asc(n2$) >= 48 And Asc(n2$) <= 57 Then
37.                     n3$ = Mid$(pCt$, 4, 1)
38.                     If Asc(n3$) >= 48 And Asc(n3$) <= Asc("6") Then
39.                         n4$ = Mid$(pCt$, 5, 1)
40.                         If Asc(n4$) >= 48 And Asc(n4$) <= 57 Then
41.                             If Val(n1$ + n2$ + n3$ + n$4) <> 0 Then rtn = TRUE
42.                         End If
43.                     End If
44.                 End If
45.             End If
46.         End If
47.  
48.     ElseIf Len(pCt$) = 7 Then
49.         '1234567
50.         'H:MM:SS
51.         If Mid$(pCt$, 2, 1) = ":" Then
52.             If Mid$(pCt$, 5, 1) = ":" Then
53.                 n1$ = Mid$(pCt$, 1, 1)
54.                 If Asc(n1$) >= 48 And Asc(n1$) <= 57 Then
55.                     n2$ = Mid$(pCt$, 3, 1)
56.                     If Asc(n2$) >= 48 And Asc(n2$) <= Asc("6") Then
57.                         n3$ = Mid$(pCt$, 4, 1)
58.                         If Asc(n3$) >= 48 And Asc(n3$) <= 57 Then
59.                             n4$ = Mid$(pCt$, 6, 1)
60.                             If Asc(n4$) >= 48 And Asc(n4$) <= Asc("6") Then
61.                                 n5$ = Mid$(pCt$, 7, 1)
62.                                 If Asc(n5$) >= 48 And Asc(n5$) <= 57 Then
63.                                     If Val(n1$ + n2$ + n3$ + n4$ + n5$) <> 0 Then rtn = TRUE
64.                                 End If
65.                             End If
66.                         End If
67.                     End If
68.                 End If
69.             End If
70.         End If
71.     End If
72.     IsCallTime = rtn
73. End Function
74.  
75.  

  [ You are not allowed to view this attachment ]  

Still can get rid of testing all digits sum <> 0 for True return.

[Jaze]

Thank you everyone. taking care of the lengths I didn't account for fixed it. And thank you for reminding me that I need to code for some of the digits between 0 and 6, not 0 to 9. Thanks again