Jump to content

My first script! Any pointers/errors/advice


Shaba
 Share

Recommended Posts

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>

Edited by Shaba
Link to comment
Share on other sites

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();}

Link to comment
Share on other sites

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

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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

Link to comment
Share on other sites

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

Edited by Shaba
Link to comment
Share on other sites

// 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.

Edited by Shaba
Link to comment
Share on other sites

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

Link to comment
Share on other sites

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))

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...