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) {
    } else {
        alert("The dragon is dead! You have won!");

do {
} while (mobHP > 0);
  • 5
    \$\begingroup\$ There is no such thing as "the most efficient way". It either fits your functional and non-functional requirements, or not. \$\endgroup\$ – zerkms May 10 '17 at 22:41
  • \$\begingroup\$ okay @zerkms are you saying the code you write matters not as long as it does exactly what we are trying to achieve? \$\endgroup\$ – Rob Hudson May 10 '17 at 22:43
  • \$\begingroup\$ The code matters if it solves the problem it was implemented for (within the given constraints). \$\endgroup\$ – zerkms May 10 '17 at 22:45
  • \$\begingroup\$ evaluateAttack could be a switch statement \$\endgroup\$ – funcoding May 10 '17 at 22:52
  • \$\begingroup\$ Your code could be more concise though, yeah. But that comes with learning language-agnostic programming techniques. \$\endgroup\$ – Gabe Rogan May 10 '17 at 22:53

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.

  1. Duplicate code like your two identical report() functions should be avoided.
  2. 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

  3. Your dice values appear fixed (ie. they do not change). Perhaps this is as intended, but I would suggest a rollDice() function is missing.

  4. 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.
  • 2
    \$\begingroup\$ Unfortunately you're not (yet) a member at SE Code Review. You should join the community here, to enjoy the rep earned. \$\endgroup\$ – πάντα ῥεῖ May 13 '17 at 17:10

I'd suggest that RPGs are good to practice Object-Oriented programming.

The initial code below will be longer, but if you are ever wanted to develop the game further, then OO style is sooo much better, and flexible. We can see why near the end...

We will build our PlayerCharacter(PC) with functionality to hold(or lose) various attack methods, and also encapsulate the Hit Points etc.

function pCharacter(heroName, charHP) {
    this.name = heroName;
    this.HP = charHP;

    // keep the attack options local to the PC: name, number of dice, dice size, attack text
    var attackMethods = [];
    attackMethods["sword"] = [2, 100, "You slash twice with your sword at the _monster_"]; 
    attackMethods["charge"] = [1, 200, "You charge the _monster_ with all your might"];
    attackMethods["spell"] = [3, 100, "You summon the fires of hell against the _monster_"];

    // so that we can add attack options later when our PC gets more powerful
    this.addAttackMethod = function(name, numDice, diceRange, desc) {
        attackMethods[name] = [numDice, diceRange, desc];

    // to return the current attack choices to the alert boxes
    // we may add a new attack option later, or lose a weapon to a monster that can disarm us!!
    this.getAttackMethods = function() {
        var str = "";
        for (var prop in attackMethods) {
            if (attackMethods.hasOwnProperty(prop)) {
                str += ", " + prop;
        str = str.substring(2, str.length);
        return str;

    keeping the attack as a PC method which takes the monster as a parameter
    then we pass damage to the monster to process.
    We could also tweak easily so that certain attacks don't damage certain monsters so much
    if we decided to pass the attack option back to the sufferDamage method of the monster.
    this.attack = function(attackMethod, monster) {
        var damage = 0;
        for (var i = 1; i <= attackMethods[attackMethod][0]; i++) {
            damage += Math.floor(Math.random() * attackMethods[attackMethod][1]) + 1;
        monster.sufferDamage(damage); //pass damage for monster to process
        alert(attackMethods[attackMethod][2].replace("_monster_", monster.type));
    this.sufferDamage = function(damage) {
        this.HP -= damage; // when you're ready to take damage from monsters!

//this constructor allows us to set up new monsters easily
function monster(monsterType, mobHP) {
    this.type = monsterType;
    this.HP = mobHP;

    this.sufferDamage = function(damage) {
        this.HP -= damage;
    this.attack = function(PC) {
    // fill out this function when you are ready to make the monsters attack the Hero

var charName = prompt("Hello Lost Stranger, Tell us your name!");
var hero = new pCharacter(charName, 1000);
var dragon = new monster("baby dragon", 500);

alert("Welcome " + hero.name + " 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.");

function attack(hero, monsterInstance) {
    // make attackgeneric function to have a battle round between the hero and a monster
    let report = function() {
        alert("You have " + hero.HP + " remaining and the " + 
              monsterInstance.type + " has " + monsterInstance.HP + " remaining.");

    var userAttack = "What type of attack will you use?(Choices: " +
                      hero.getAttackMethods() + ")"; //get current attack options
    var attackType = prompt(userAttack);

    hero.attack(attackType, monsterInstance);

    if (monsterInstance.HP > 0) {
    } else {
        alert("The " + monsterInstance.type + " is dead! You have won!");

do {
    attack(hero, dragon);
} while (dragon.HP > 0);

Your program ended here - below you can see how easy it is to continue the game with new monsters and adding more powerfull weapons to the hero. The new Battleaxe now show up in the list of attack choices. It rolls 6 * dice(150) to do damage!

hero.addAttackMethod("battleaxe", 6, 150, 
                     "You swing the enchanted Mighty BattleAxe, crashing it into the _monster_!");
alert("You gain the Mighty BattleAxe!");

alert("A massive intimidating dragon lord appears!! It does not look happy.");

// now see how easy to have the hero fight a new monster
var masterDragon = new monster("Dragon Lord", 3000);
do {
    attack(hero, masterDragon);
} while (masterDragon.HP > 0);

There were better techniques to use. You can use to retain the attack method in a 'closure', and then the monster can check what attack type was used. I didn't do it.

You can have the monster fight back and have the PC react defensively too!

This is just a start of how you could set things up...


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.