HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Slaying a dragon, the old-school way

Submitted by: @import:stackexchange-codereview··
0
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?

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.

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