Any idea why my JS function isn't working?

  • Thread starter Thread starter Jamin2112
  • Start date Start date
  • Tags Tags
    Function Idea
AI Thread Summary
The JavaScript function designed to convert time from hh:mm:ss format to seconds is not functioning correctly, returning incorrect values for certain inputs. Key issues include the need to specify the radix parameter in the parseInt function to avoid misinterpretation of leading zeros. Suggestions for improvement include consolidating error checks into a single function and ensuring proper syntax by adjusting substring parameters. After debugging, the function was successfully corrected, passing tests for various time formats. The final implementation demonstrates a more elegant approach to handling time conversion.
Jamin2112
Messages
973
Reaction score
12
I made this to convert a time in hh:mm:ss form to seconds. Not working, for some reason. I tested "00:02:29" and got "2". Any idea why? Also, any ideas on how I can make this procedure more simple and elegant?

Code: 
function clock2secs(clockTime)
{ 
		
	if (clockTime.length > 8)
	{
		return -1;
	}
			
	if (isDecimal(clockTime.charAt(0)) && isDecimal(clockTime.charAt(1))) 
	{
		var hrs = parseInt(clockTime.substring(0,1));
	} else 
	{
		return -1;
	}
			
	if (clockTime.charAt(2) != ':')
	{
		return -1;
	}
			
	if (isDecimal(clockTime.charAt(3)) && isDecimal(clockTime.charAt(4))) 
	{
		var mins = parseInt(clockTime.substring(3,4));
	} else 
	{
		return -1;
	}
			
	if (clockTime.charAt(5) != ':')
	{
		return -1;
	}
			
	if (isDecimal(clockTime.charAt(6)) && isDecimal(clockTime.charAt(7))) 
	{
		var secs = parseInt(clockTime.substring(6,7));
	} else 
	{
		return -1;
	}
			
	/* If here, clockTime was in valid format hh:mm:ss, and its
	   minutes and seconds were parsed correctly.
	*/
	return 3600 * hrs + 60 * mins + secs;
}

function isDecimal(c) 
{
	return c <= '9' && c >= '0';
}
 
Last edited by a moderator:
Technology news on Phys.org
I would use the radix parameter in the parseInt calls. I am not sure how parseInt would interpret "08" or "09" without it.
Otherwise, it looks fine. ... But I'll continue looking for the bug.
I would have done all of the checking before any of the parseInt's. That way you could put all the if statements under a comment that says "Check proper syntax" and the remaining statements under a separate heading.

Oh. I would definitely change the first check to "is not equal to 8".
 
I took a ton of tabs out of your code since some lines were wider than the display window.
Jamin2112 said:
I made this to convert a time in hh:mm:ss form to seconds. Not working, for some reason. I tested "00:02:29" and got "2". Any idea why? Also, any ideas on how I can make this procedure more simple and elegant?

Code: 
function clock2secs(clockTime)
{ 
		
	if (clockTime.length > 8)
	{
		return -1;
	}
			
	if (isDecimal(clockTime.charAt(0)) && isDecimal(clockTime.charAt(1))) 
	{
		var hrs = parseInt(clockTime.substring(0,1));
	} else 
	{
		return -1;
	}
			
	if (clockTime.charAt(2) != ':')
	{
		return -1;
	}
			
	if (isDecimal(clockTime.charAt(3)) && isDecimal(clockTime.charAt(4))) 
	{
		var mins = parseInt(clockTime.substring(3,4));
	} else 
	{
		return -1;
	}
			
	if (clockTime.charAt(5) != ':')
	{
		return -1;
	}
			
	if (isDecimal(clockTime.charAt(6)) && isDecimal(clockTime.charAt(7))) 
	{
		var secs = parseInt(clockTime.substring(6,7));
	} else 
	{
		return -1;
	}
			
	/* If here, clockTime was in valid format hh:mm:ss, and its
	   minutes and seconds were parsed correctly.
	*/
	return 3600 * hrs + 60 * mins + secs;
}

function isDecimal(c) 
{
	return c <= '9' && c >= '0';
}
There are a couple of things you can do to debug.
1. Add in some alert() calls, with the value of the variable you're interested in. This causes a window to pop open to display that value.
2. Use the F12 debugging tools that are available with most modern browsers. These debuggers allow you to set a breakpoint and single-step through your code. They will also show you the local variables and their values.
 
To add to what .Scott said about putting all the sanity check code in one place (like possibly in its own function), you could combine these two if blocks:
Code: 
if (clockTime.charAt(2) != ':')
{
	return -1;
}

Code: 
if (clockTime.charAt(5) != ':'  )
{
	return -1;
}

into one if block, like so:
Code: 
if (clockTime.charAt(2) != ':' || clockTime.charAt(5) != ':')
{
	return -1;
}
 
The problem is with you "substring" parameters. You need to add one to all of you terminating indeces.
 
Also, compare your results for "01:01:01" with "09:09:09". Just to demonstrate the problem when you don't specify decimal to parseInt.
 
.Scott said:
That way you could put all the if statements under a comment that says "Check proper syntax" and the remaining statements under a separate heading.

I always thought that it was proper coding to check for errors while in the process of trying to accomplish a task.
 
There's nothing that says you can't check for the errors all at once, and if you don't find any, carry on with the task at hand.
 
.Scott said:
Also, compare your results for "01:01:01" with "09:09:09". Just to demonstrate the problem when you don't specify decimal to parseInt.

It should work as is. Look at the specifications of parseInt() http://www.w3schools.com/jsref/jsref_parseint.asp
 
  • #10
Jamin2112 said:
I always thought that it was proper coding to check for errors while in the process of trying to accomplish a task.
Either way is proper coding. The issue is one of presentation - and to a minor extent modularity.
When you code, you can think of what words you would use to explain what your code is doing. My thought is that it is more awkward and overly detailed to talk about each token in the string - and more fluent to describe it as a two step process, check & convert.

But either way is completely "proper".

The other aspect is how you demonstrate your defenses. When you write a function, you often treat the input parameters as coming straight from the devil whose sole purpose is to torture the world through your function. Your defense is either to check all parameters upon entry - or to check each parameter just before using it. In the first case, it is easier to check whether your defense is adequate because all of the checks are done in one place.
 
  • #11
Mark44 said:
There's nothing that says you can't check for the errors all at once, and if you don't find any, carry on with the task at hand.

So, this looks good to you?

Code: 
function clock2secs(clockTime)
{
	if (isProperTimeForm(clockTime))
	{ 
		var hrs = parseInt(clockTime.substring(0,2));
		var mins = parseInt(clockTime.substring(3,5));
		var secs = parseInt(clockTime.substring(6,8));
		return 3600 * hrs + 60 * mins + secs;
	} else 
	{
		return -1;
	}
}

 function isProperTimeForm(clockTime)
{
	   return          (clockTime.length == 8)
			&& (isDecimal(clockTime.charAt(0)))
			&& (isDecimal(clockTime.charAt(1)))
			&& (clockTime.charAt(2) == ':')
			&& (isDecimal(clockTime.charAt(3))
			&& (isDecimal(clockTime.charAt(4))
			&& (clockTime.charAt(5) == ':')
			&& (isDecimal(clockTime.charAt(6))
			&& (isDecimal(clockTime.charAt(7));
}

function isDecimal(c) 
{
	return c <= '9' && c >= '0';
}
 
Last edited:
  • #13
Jamin2112 said:
So, this looks good to you?
I like it - or I will after you make that "09:09:09" check.

--------------------------

Actually, I just looked it up. The "09:09:09" check may or may not reveal the problem. Here is the issue:
If the radix parameter is omitted, JavaScript assumes the following:

If the string begins with "0x", the radix is 16 (hexadecimal)
If the string begins with "0", the radix is 8 (octal). This feature is deprecated
If the string begins with any other value, the radix is 10 (decimal)
 
  • #14
.Scott said:
Humor me and check it with "09:09:09".

parseInt("09:09:09") == "9",

if that's what you were getting at.

However, I'm only using parseInt() on each the 3 substrings separated by colons.
 
  • #15
Obviously "9" is not the answer. But it's also not what I would have expected.
I would have expected that with the latest browsers you would have gotten the correct answer (32949) and with other browsers you would have gotten zero.

Did you put the ",10" in the last parseInt but not the first two?
 
  • #16
It turns out I can't do multiple-line statements like this:

Code: 
	return             (clockTime.length == 8) 
			&& (isDecimal(clockTime.charAt(0))) 
			&& (isDecimal(clockTime.charAt(1)))
			&& (clockTime.charAt(2) == ':')
			&& (isDecimal(clockTime.charAt(3))
			&& (isDecimal(clockTime.charAt(4))
			&& (clockTime.charAt(5) == ':')
			&& (isDecimal(clockTime.charAt(6))
			&& (isDecimal(clockTime.charAt(7));

Dang it.
 
  • #17
Jamin2112 said:
So, this looks good to you?

Code: 
			&& (isDecimal(clockTime.charAt(0)))
			&& (isDecimal(clockTime.charAt(1)))
			&& (clockTime.charAt(2) == ':')
			&& (isDecimal(clockTime.charAt(3))
			&& (isDecimal(clockTime.charAt(4))
			&& (clockTime.charAt(5) == ':')
			&& (isDecimal(clockTime.charAt(6))
			&& (isDecimal(clockTime.charAt(7));
You're missing some end parenthesis.
 
  • #18
Jamin2112 said:
It turns out I can't do multiple-line statements like this: ...
Dang it.
Don't dang it. Put the end parens in. You are missing four of them.

-------------------------

And let us know how you're doing.
 
Last edited:
  • #19
Wow. People as stupid as me should not be allowed to live.

I got it to work, finally.

This is one of my first times actually using JavaScript. I'll show you the full idea for my program if I run into any more trouble.

Test of the clocks2sec function:

Code: 
document.write(clock2secs("00:02:29"))

function clock2secs(clockTime)
{
	if (isProperTimeForm(clockTime))
	{ 
		var hrs = parseInt(clockTime.substring(0,2));
		var mins = parseInt(clockTime.substring(3,5));
		var secs = parseInt(clockTime.substring(6,8));
		return 3600 * hrs + 60 * mins + secs;
	} 
	else 
	{
		return -1;
	}
}

 function isProperTimeForm(clockTime)
{
	return     (clockTime.length == 8) 
			&& (isDecimal(clockTime.charAt(0))) 
			&& (isDecimal(clockTime.charAt(1)))
			&& (clockTime.charAt(2) == ':')
			&& (isDecimal(clockTime.charAt(3)))
			&& (isDecimal(clockTime.charAt(4)))
			&& (clockTime.charAt(5) == ':')
			&& (isDecimal(clockTime.charAt(6)))
			&& (isDecimal(clockTime.charAt(7)));
}function isDecimal(c) 
{
	return c <= '9' && c >= '0';
}

produces

Code: 
149
 
  • #20
Jamin2112 said:
Wow. People as stupid as me should not be allowed to live.
I got it to work, finally.
If you want to feel smart, stay away from programming.
Become a Supreme Court Justice. They're never wrong.

-------

You are going to redo the "09:09:09" test, I hope.
 
  • #21
.Scott said:
You are going to redo the "09:09:09" test, I hope.

It passes that test.
 
  • #22
Jamin2112 said:
It passes that test.
That's fine. But with some browsers it won't work. In fact with most browsers currently in use it won't work. The reason is that the original definition for parseInt was that a leading zero meant that an octal value was being specified. So once it hit an "8" or "9" after that, it would stop.
 
  • #23
.Scott said:
That's fine. But with some browsers it won't work. In fact with most browsers currently in use it won't work. The reason is that the original definition for parseInt was that a leading zero meant that an octal value was being specified. So once it hit an "8" or "9" after that, it would stop.

You're right. I tried it in Internet Explorer and it didn't work. It works in Firefox and Chrome, though.
 
  • #24
Jamin2112 said:
You're right. I tried it in Internet Explorer and it didn't work. It works in Firefox and Chrome, though.
And you know the fix. Right?
 
  • #25
.Scott said:
And you know the fix. Right?

Yea, specify the radix
 
  • #26
Any idea why my border color isn't working when I test my table?


Code: 
<html>

	<head>

		<title>Practice site</title>
		
		<style type="text/css">
			#convtbl
			{
				border-color: black;
				background-color: yellow;
			}
			
			td.iptcell
			{
				color: blue;
			}
			
		</style>

	</head>

<body>

<center>

	<table id="convtbl">
	   <tr>
		  <td>Distance 1</td>
		  <td>Time 1</td>
		  <td>Distance 2</td>
		  <td>Time 2</td>
	   </tr>
	   <tr>
		  <td class="iptcell" id="d1">&nbsp;</td>
		  <td class="iptcell" id="t1">&nbsp;</td>
		  <td class="iptcell" id="d2">&nbsp;</td>
		  <td class="iptcell" id="t2">&nbsp;</td>
	   </tr>
	</table>


<script type="text/javascript">
		
	//<button type="button" onclick="calcTimeEquiv()">Convert Running Times</button>
	document.write(clock2secs("09:09:09"));
	
	function clock2secs(clockTime)
	{
		if (isProperTimeForm(clockTime))
		{ 
			var hrs = parseInt(clockTime.substring(0,2), 10);
			var mins = parseInt(clockTime.substring(3,5), 10);
			var secs = parseInt(clockTime.substring(6,8), 10);
			return 3600 * hrs + 60 * mins + secs;
		} 
		else 
		{
			return -1;
		}
	}
	
	 function isProperTimeForm(clockTime)
	{
		return     (clockTime.length == 8) 
				&& (isDecimal(clockTime.charAt(0))) 
				&& (isDecimal(clockTime.charAt(1)))
				&& (clockTime.charAt(2) == ':')
				&& (isDecimal(clockTime.charAt(3)))
				&& (isDecimal(clockTime.charAt(4)))
				&& (clockTime.charAt(5) == ':')
				&& (isDecimal(clockTime.charAt(6)))
				&& (isDecimal(clockTime.charAt(7)));
	}
	 
	function isDecimal(c) 
	{
		return c <= '9' && c >= '0';
	}
	
	function tdconvert()
	{
	
	
	}
		
</script>
</center>
</body>

</html>
 
  • #27
What's your favorite border-style?
---------------------
edit...
If you haven't taken that hint yet, my favorite is "border-style: solid;".
 
Last edited:
  • #28
Any idea why this opposite function isn't working? Any idea how I can shorten it, eliminate repeated logic (if there is any)?

I tried feeding it "183" and was expecting it to output "00:03:03" but instead it gave me "0.050833333333333335:0.0008472222222222222:0.00001412037037037037"





Code: 
	function secs2Clock(secsTime)
	{
		/* Return hh:mm::ss form string representation of seconds secsTime.
		   Checks that the input string secsTime is of the correct form, 
		   a positive base-10 number. Return -1 on error. */
		var n = secsTime.length;
		if (n == 0)
		{
			return -1;
		}
		for (var k = 0; k < n; k++)
		{
			if (!isDecimal(secsTime.charAt(k)))
			{
				return -1;
			}
		}
		
		var secs = parseInt(secsTime);
		
		var maxTime = 99 * 3600 + 59 * 60 + 59; // Maximum seconds allowed: 99:59:59 
		if (secs > maxTime)
		{
			return -1;
		}
		
		var hh = (secs / 3600).toString();
		if (hh.length == 1)
		{
			hh = "0" + hh;
		}
		secs /= 3600;
		var mm = (secs / 60).toString();
		if (mm.length == 1)
		{
			mm = "0" + mm;
		}
		secs /= 60;
		var ss = (secs / 60).toString();
		if (ss.length == 1)
		{
			ss = "0" + ss;
		}
		return hh + ":" + mm + ":" + ss;
	}
 
  • #29
Whoops ... This is better:

Code: 
	function secs2Clock(secsTime)
	{
		/* Return hh:mm::ss form string representation of seconds secsTime.
		   Checks that the input string secsTime is of the correct form, 
		   a positive base-10 number. Return -1 on error. */
		var n = secsTime.length;
		if (n == 0)
		{
			return -1;
		}
		for (var k = 0; k < n; k++)
		{
			if (!isDecimal(secsTime.charAt(k)))
			{
				return -1;
			}
		}
		
		var ss = parseInt(secsTime);
		
		var maxTime = 99 * 3600 + 59 * 60 + 59; // Maximum seconds allowed: 99:59:59 
		if (ss > maxTime)
		{
			return -1;
		}
		
		var hh = (Math.floor(ss / 3600)).toString();
		if (hh.length == 1)
		{
			hh = "0" + hh;
		}
		ss %= 3600;
		var mm = (Math.floor(ss / 60)).toString();
		if (mm.length == 1)
		{
			mm = "0" + mm;
		}
		ss = (ss % 60).toString();
		if (ss.length == 1)
		{
			ss = "0" + ss;
		}
		return hh + ":" + mm + ":" + ss;
	}
 
  • #30
This thread is like a tutorial on the joys and terrors of Javascript. Javascript programming is particularly challenging because of browser differences and because it is a mongo huge language packed with all kinds of capabilities that people generally don't know or understand, and thus, which can trip us up in seemingly simple code. Thus, I highly recommend anyone working regularly with Javascript to study Douglas Crockford's book "Javascript: The Good Parts" , in which he catalogs the "dangerous" features within Javascript and describes how it's object-orientation differs from other common languages (that JS superficially resembles syntactically), and how it even has some functional programming capabilities. Here's the link on Amazon: https://www.amazon.com/dp/0596517742/?tag=pfamazon01-20

The book is not a full-fledged reference, but rather, a catalog of best practices and things to avoid.
 
Last edited by a moderator:
  • #31
I second harborsparrow's recommendation of "JavaScript: The Good Parts". For reference, I also have David Flanagan's "JavaScript: The Definitive Guide", 6th Ed.

Jamin2112, I don't know if you took me up on my suggestion to use the F12 debugging tools (available in pretty much any browser). Debugging your code using these capabilities would provide answers to many of the questions you've asked.

There is also the timeworn method of using print statements to display the values of variables at certain locations in your code. You can do this by adding alert(...) statements in your code.
 

Similar threads

Why isn't my algorithm code working?
Replies
4
Views
2K
Python Why is my Hydrodynamics Simulation Code Producing a Noisy Signal?
Replies
3
Views
2K
Java Why Won't My Java Blackjack Game Compile?
Replies
3
Views
5K
Why Is My Runge Kutta Implementation Not Working for Car Simulation?
Replies
12
Views
3K
Why Does My C Program Segmentation Fault Without a Filename?
Replies
5
Views
3K
Fortran Why Am I Getting an 'Out of Range in G Table' Error in FORTRAN?
Replies
1
Views
3K

Hot Threads

Recent Insights

Back
Top