Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type. This has a significant compile-time advantage since it allows for better type-checking but it has a run-time disadvantage run-time disadvantage. (Although I don't think you need to be worried about the run-time disadvantage)

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type. This has a significant compile-time advantage since it allows for better type-checking but it has a run-time disadvantage. (Although I don't think you need to be worried about the run-time disadvantage)

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type. This has a significant compile-time advantage since it allows for better type-checking but it has a run-time disadvantage. (Although I don't think you need to be worried about the run-time disadvantage)

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }
added 286 characters in body
Source Link
Simon Forsberg
  • 59.7k
  • 9
  • 157
  • 311

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type. This has a significant compile-time advantage since it allows for better type-checking but it has a run-time disadvantage . (Although I don't think you need to be worried about the run-time disadvantage)

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type.

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type. This has a significant compile-time advantage since it allows for better type-checking but it has a run-time disadvantage . (Although I don't think you need to be worried about the run-time disadvantage)

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }
Source Link
Simon Forsberg
  • 59.7k
  • 9
  • 157
  • 311

To be consistent with your variable names, I would rename Trucks to lowercase trucks.

You have several for-loops that are very similar, for creating Trucks and LogWoods. Consider creating a function createTruck(x, y, speed); the implementation could also take care of adding the truck to the corresponding Array and adding the child to the MovieClip. That way, you could rewrite a for-loop like this:

 for (var i=1; i<=2; i++)
 {
 createTruck(200 * i) + 350, trucksYPos[3], -5);
 }

And by doing this you also don't need the newTruck variable, as that will only exist within the createTruck function.

Instead of using the type Array, I suggest you use the Vector class which has the advantage of being typed so it only allows adding objects of a specific type.

I am curious about how you use your startWin and startLose methods. I assume they're being called from a timeline somewhere. All they do is to add an event listener for the click of a button. Of course this is important, but should it have it's own method? Especially considering they do the exact same thing it feels strange. As soon as your btnBack exists (is not null), add the event listener, and do it only once. I don't see the need for two separate methods.

Using a switch-statement in your keyDownHandler method would reduce your line count and improve the readability of your code.

 switch (evt.keyCode) {
 case Keyboard.LEFT:
 p1speedX = -1;
 break;
 case Keyboard.RIGHT:
 p1speedX = 1;
 break;
 case Keyboard.UP:
 p1speedY = -1;
 break;
 case Keyboard.DOWN:
 p1speedY = 1;
 break;
 }
default

AltStyle によって変換されたページ (->オリジナル) /