My first script! Any pointers/errors/advice

[Shaba]

Hi,I'm new to the forum but have been learning HTML,CSS and Javascript (Mainly JS I have a basic knowledge in the other two) from W3 for a few day or two and have just got my first script finished (Or so I think).Would any one be able to look over it and may be point out any bad coding/errors/tweeks I can look at doing and learning how and why to do them.Thank you all!

<script type="text/javascript">// JavaScript Document////	Copyright 2010//		////	 Order Cut-Off, count down timer 3PM Every Day//		-Friday Orders delivered Monday if ordered before 3pm//		-Weekend Orders dispatched Monday - Delivery Tuesdayvar d=new Date();theDay=d.getDay();switch (theDay){case 1:  cutofftime(0);  break;case 2:  cutofftime(0);  break;case 3:  cutofftime(0);  break;case 4:  cutofftime(0);  break;case 5:  friday(0);  break;case 6:  weekend(0);  break;case 0:  weekend(0);  break;  default:  document.write("-");}function cutofftime(cutoff){var cutoff = new Array(2)cutoff[0] = "You can still order for next day delivery!"cutoff[1] = "The courier has collected for today, your order will be dispatched tomorrow"			var d=new Date();		var time=d.getHours();						if (time < 15) 		document.write(cutoff[0])				else		document.write(cutoff[1])}function friday(friday){var friday = new Array(2)friday[0] = "Order now and your item will be dispatch today for delivery Monday!"friday[1] = "The courier has collected for today, your order will be dispatched Monday."			var d=new Date();		var time=d.getHours();						if (time < 15) 		document.write(cutoff[0])				else		document.write(cutoff[1])}function weekend(weekend){var weekend = new Array(2)weekend[0] = "Order now and your item will be dispatched Monday for delivery Tuesday."weekend[1] = "Order now and your item will be dispatched Monday for delivery Tuesday."			var d=new Date();		var time=d.getHours();						if (time < 15) 		document.write(cutoff[0])				else		document.write(cutoff[1])}		</script>

[jeffman]

There are quite a few things that can be combined. Let's start with your switch structure. Notice how normal case structures end with a break statement. If the break were not there, execution would fall through to the next case structure. Adding a break keeps this from happening.BUTSometimes that's a good thing. If the statements in multiple, sequential case structures are the same, they can all be combined. So if I have this code:

switch (x){case 1:  jump();  break;case 2:  jump();  break;case 3:  jump();  break;case 4:  sleep(0);  break;default:  wake();}

The first three cases do the same thing. So they can be combined:

switch (x){case 1:case 2:case 3:  jump();  break;case 4:  sleep(0);  break;default:  wake();}

[Shaba]

Hi Deirdre's Dad,Thank you for the quick reply. I have looked over the script and made the following changes based on your advise:

var d=new Date();theDay=d.getDay();switch (theDay){case 1: cutofftime(0);case 2: cutofftime(0);case 3: cutofftime(0);case 4: cutofftime(0);  break;case 5:  friday(0);  break;case 6: weekend(0);case 0: weekend(0);  break;  default:  document.write("ERROR");}

I assume removing the breaks would speed up the rate the script is read (This maybe a small script but on a larger scale) and makes it clearer overall.I appreciate you taking the time to reply.Many Thanks,Kyle

[jeffman]

Actually, the speed you gain is download time. What the interpreter does isn't really different. With or without the breaks, it still evaluates each case statement, kinda like:is it 1? no. move onis it 2? no. move onetc. You also benefit from a script that is easier to read. In a world where processing and transmission speeds are high, and hard disc space is almost unlimited, development speed is often the costliest element.

[jeffman]

Next suggestion: your friday, cutofftime, and weekend functions could easily be combined into a single function. The change would involve passing it different arguments, depending on the type of day, and combining all your text statements into a single array. The value of the argument would determine which set of array values you write.

[Shaba]

Right I see, that makes sense. Try and keep it simple so as not to confuse and make things more complicated then they need be (result = faster working pace). :)I am at work at the moment but I will tackle your next challenge once I get home. I will give it a try and see what happens I'm just not sure about how I would go about making it detect the day and time in one function then pass it onto the correct array, I just can't think how to handle all the variables.Anyway I won't start asking questions without trying to do it first, I will report back tomorrow with some good news I hope :)Thanks again for your time and help,Kyle

[Shaba]

Next suggestion: your friday, cutofftime, and weekend functions could easily be combined into a single function. The change would involve passing it different arguments, depending on the type of day, and combining all your text statements into a single array. The value of the argument would determine which set of array values you write.

I had a look over the script after reading a few more tutorials and can now see the way forward I think, I have come up with this script:

<script type="text/javascript">// JavaScript Document////	Copyright 2010//		////	 Order Cut-Off, count down timer 3PM Every Day//		-Friday Orders delivered Monday if ordered before 3pm//		-Weekend Orders dispatched Monday - Delivery Tuesdayvar d=new Date();theDay=d.getDay();function cutofftime(){var cutoff = new Array(2)cutoff[0] = "You can still order for next day delivery!"cutoff[1] = "The courier has collected for today, your order will be dispatched tomorrow"cutoff[3] = "Order now and your item will be dispatch today for delivery Monday!"cutoff[4] = "Order now and your item will be dispatched Monday for delivery Tuesday."			var d=new Date();		var time=d.getHours();						if (time<15 && theDay==(1||2||3||4))		{		document.write(cutoff[0])		}				else if (time<15 && theDay==5)		{		document.write(cutoff[3])		}				else if (theDay==(0||6))		{		document.write(cutoff[4])		}				else		{		document.write(cutoff[1])		}}		</script><script Language="JavaScript"> // This will be in bodycutofftime();</Script>

I hope this is better, it looks a lot cleaner and is much more compact. Cheers,Kyle

[Shaba]

// EDIT - Made a few mistakes in the last one.I have cracked it I think:

<script type="text/javascript">// JavaScript Document////	Copyright 2010//		////	 Order Cut-Off, count down timer 3PM Every Day//		-Friday Orders delivered Monday if ordered before 3pm//		-Weekend Orders dispatched Monday - Delivery Tuesdayvar d=new Date();theDay=d.getDay();function cutofftime(){var cutoff = new Array(2)cutoff[0] = "You can still order for next day delivery!"cutoff[1] = "The courier has collected for today, your order will be dispatched tomorrow"cutoff[3] = "Order now and your item will be dispatch today for delivery Monday!"cutoff[4] = "Order now and your item will be dispatched Monday for delivery Tuesday."			var d=new Date();		var time=d.getHours();						if (time<15 && theDay==1 || theDay==2 || theDay==3 || theDay==4)		{		document.write(cutoff[0])		}				else if (time<15 && theDay==5)		{		document.write(cutoff[3])		}				else if (theDay==0 || theDay==6)		{		document.write(cutoff[4])		}				else		{		document.write(cutoff[1])		}}		</script><script Language="JavaScript"> // This will be in bodycutofftime();</Script>

This seems to work, I have tested for each day just not the afternoon as yet.

[jeffman]

You've made some good progress. Looks like you have some programming instinct. Keep at it.

[Shaba]

Seems to work as far as I can tell! :)Thank you Deirdre's Dad, you have really helped me progress through these last few lessons I had just hit a bit of a "mind block" but I now have a better knowledge of the whole concept and how to structure my script. On to the next step then I have read that using document.write is not good practice so I am going to look into changing that tonight to however its recommended to be done now.So expect a message from me in the morning lol Joking apart I really appreciate your help, a kick in the right direction is all we need sometimes Cheers,Kyle

[thescientist]

why not just write this line

if (time<15 && theDay==1 || theDay==2 || theDay==3 || theDay==4)

like this

if (time<15 && (theDay > 0 && theDay <= 4))