patternjavascriptMinor
Slaying a dragon, the old-school way
Viewed 0 times
theslayingwayschooldragonold
Problem
Is the below program written in the most efficient way possible? I am new to Java Script and am just trying to make something basic so I can practice as I learn more.
The way I wrote it the program does work (except it keeps wanting to run again after it's over?) but is there a better way I could have accomplished this?
The way I wrote it the program does work (except it keeps wanting to run again after it's over?) but is there a better way I could have accomplished this?
var charName = prompt("Hello Lost Stranger, Tell us your name!");
alert("Welcome " + charName + " we have been waiting for someone who can challenge this dragon!");
alert("A rather small but still intimidating dragon appears; it does not look happy.");
var charHP = 3000;
var mobHP = 3000;
var userAttack = "What type of attack will you use?(Choices: Sword, Charge, Spell)";
var diceOne = Math.floor(Math.random() * 100) + 1;
var diceTwo = Math.floor(Math.random() * 100) + 1;
var diceThree = Math.floor(Math.random() * 100) + 1;
function report() {
alert("You have " + charHP + " remaining and the dragon has " + mobHP + " remaining.");
}
function attack() {
function report() {
alert("You have " + charHP + " remaining and the dragon has " + mobHP + " remaining.");
}
var attackType = prompt(userAttack);
function evaluateAttack() {
if (attackType.toLowerCase() === "sword") {
mobHP = mobHP - diceOne - diceTwo - diceThree;
alert("You slash at the dragon three times drawing blood!");
} else if (attackType.toLowerCase() === "charge") {
mobHP = mobHP - (3 * diceOne);
alert("You charge the dragon at full speed drawing blood!");
} else if (attackType.toLowerCase() === "spell") {
mobHP = mobHP - (4 * diceTwo);
alert("You close your eyes and summon the fires of hell on the dragon");
}
}
if (mobHP > 0) {
report();
evaluateAttack();
} else {
alert("The dragon is dead! You have won!");
}
}
do {
attack();
} while (mobHP > 0);Solution
Ok, the following isn't necessarily about making your code more efficient (in the sense it executes faster), just a couple of quick observations/pointers.
be avoided.
-
Your if..else if..else if statement may be better suited as a switch()/case. Under some conditions, switch() can be more efficient than if/else-if/.. (depending on the javascript engine, etc.) I should note that developers have been fighting the "switch-vs-if-else-if war for a while now. https://stackoverflow.com/questions/2922948/javascript-switch-vs-if-else-if-else
-
Your dice values appear fixed (ie. they do not change). Perhaps this is as intended, but I would suggest a rollDice() function is missing.
- Duplicate code like your two identical report() functions should
be avoided.
-
Your if..else if..else if statement may be better suited as a switch()/case. Under some conditions, switch() can be more efficient than if/else-if/.. (depending on the javascript engine, etc.) I should note that developers have been fighting the "switch-vs-if-else-if war for a while now. https://stackoverflow.com/questions/2922948/javascript-switch-vs-if-else-if-else
-
Your dice values appear fixed (ie. they do not change). Perhaps this is as intended, but I would suggest a rollDice() function is missing.
- Closures (nested functions, or "functions within functions" if you like) may be a difficult concept for beginners... It can introduce problems with variable scope that can be difficult to understand/deal with. Disclaimer: I am not saying closures are bad, they are powerful - but perhaps overkill for your current purposes, that's all.
Context
StackExchange Code Review Q#163266, answer score: 6
Revisions (0)
No revisions yet.