Any idea why my JS function isn't working?

In summary, the conversation revolves around a code designed to convert a time in the format of hh:mm:ss to seconds. The code is not working properly and is only outputting "2" when tested with "00:02:29". The experts discuss possible errors in the code and suggest using different methods of checking for errors and converting the time. They also mention the importance of using the radix parameter in the parseInt function to avoid errors with numbers starting with 0.
  • #1
Jamin2112
986
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
  • #2
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".
 
  • #3
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.
 
  • #4
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;
}
 
  • #5
The problem is with you "substring" parameters. You need to add one to all of you terminating indeces.
 
  • #6
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.
 
  • #7
.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.
 
  • #8
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.
 
  • #9
.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.
 

1. Why is my function not returning the expected result?

There could be several reasons why your function is not returning the expected result. Some common issues include syntax errors, incorrect variable names or values, and logic errors in your code. It is important to carefully review your code and use debugging tools to identify and fix any errors.

2. How do I troubleshoot my JavaScript function?

To troubleshoot your JavaScript function, you can use debugging tools such as console.log() statements or browser developer tools. These tools allow you to track the flow of your code and identify any errors or unexpected behavior. You can also try breaking down your function into smaller parts and testing each part separately to pinpoint the issue.

3. Can my function be affected by external factors?

Yes, your JavaScript function can be affected by external factors such as user input, browser compatibility, and changes to external resources. It is important to consider these factors when troubleshooting your function and to test your code in different environments to ensure it works as intended.

4. How can I improve the performance of my function?

To improve the performance of your JavaScript function, you can optimize your code by using efficient algorithms, minimizing the use of global variables, and avoiding unnecessary loops or recursive calls. You can also consider using built-in JavaScript functions or libraries to perform complex tasks more efficiently.

5. Is there a limit to the complexity of a JavaScript function?

There is no specific limit to the complexity of a JavaScript function, but it is generally recommended to keep your code simple and easy to understand. Breaking down complex tasks into smaller, reusable functions can also improve the readability and maintainability of your code. Additionally, some browsers may have limitations on the amount of memory or processing power available, so it is important to consider performance when writing complex functions.

Similar threads

  • Programming and Computer Science
Replies
3
Views
2K
  • Engineering and Comp Sci Homework Help
Replies
4
Views
2K
  • Programming and Computer Science
Replies
3
Views
5K
  • Engineering and Comp Sci Homework Help
Replies
1
Views
2K
  • Programming and Computer Science
Replies
1
Views
2K
Back
Top