Pokemon game in JavaScript I'm practicing my JavaScript skills by trying to create a Pokemon game. So far, I've made the dataset and the functions required to work the game. Can someone help me optimize the code and tell me why they made that change?

function pokemon(name, hp, critical_attack, lowest_attack, highest_attack) {
    this.name = name;
    this.hp = hp;
    this.critical_attack = critical_attack;
    this.lowest_attack = lowest_attack;
    this.highest_attack = highest_attack;   
}

var Charizard = new pokemon("Charizard", 90, 30, 1, 15);
var Blastoise = new pokemon("Blastoise", 95, 28, 1, 14);
var Venasaur = new pokemon("Venasaur", 100, 26, 1, 13);

var poke_choice = new Array ("Charizard", "Blastoise", "Venasaur")

function player(type, name, pokemon) {
    this.type = type;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player("User", "Arhum", "Charizard");
console.log(window[user.pokemon].hp)

for(i = 0; i&lt;= 2; i++) {
  if (poke_choice[i] === user.pokemon) {
    poke_choice.splice(i,1);
  }
}  

var comp_choice = function() {
  return poke_choice[Math.round(Math.random())]
}

var comp = new player("Computer","Red", comp_choice());

var user_attack = function() {
  var uhigh = window[user.pokemon].highest_attack
  var ulow = window[user.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*uhigh)
}        

var comp_attack = function() {
  var chigh = window[comp.pokemon].highest_attack
  var clow = window[comp.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*chigh)
}     

console.log(window[comp.pokemon].name)
console.log(window[user.pokemon].name)
console.log(user_attack())
console.log(comp_attack())
function player(type, name, pokemon) {
    this.type = type;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player("User", "Arhum", "Charizard");
console.log(window[user.pokemon].hp)



There's some really strange and bad practices going on here, especially with the pokemon field. Here, you are relying on the fact that the string passed in will correspond with some variable on the global scope that is hopefully of the type pokemon.

That's not the right way to do things. Instead, player objects should be instantiated with the actual pokemon object itself:

var user = new player("User", "Arhum", Charizard);


Then, whenever you need to access information about the player's pokemon, rather than doing this:

window[user.pokemon]


You just do this:

user.pokemon


This is a lot safer, and follows practices much better.




var user_attack = function() {
  var uhigh = window[user.pokemon].highest_attack
  var ulow = window[user.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*uhigh)
}        

var comp_attack = function() {
  var chigh = window[comp.pokemon].highest_attack
  var clow = window[comp.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*chigh)
}



These two functions do exactly the same exact thing, except they are for two very specific instances of a class. This is also terrible practice, as nothing is flexible here and functions not only repeat themselves but rely on two very specific objects to be, again, in the global scope.

Instead, these two functions should become one instance method of the player class:

player.prototype.attack = function() {
  var uhigh = this.pokemon.highest_attack
  var ulow = this.pokemon.lowest_attack
  return Math.round(Math.random().toFixed(1)*uhigh)
}


Then, you should use this methods instead of those other two functions.

user.attack();
comp.attack();


You may want to rename to doAttack as the wording can be confusing.




function player(type, name, pokemon) {
    this.type = type;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player("User", "Arhum", "Charizard");



Are there any types other than "User" and "Computer"? If not, then you should turn this property into a boolean value something like this:

function player(isComputer, name, pokemon) {
    this.isComputer = isComputer;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player(false, "Arhum", "Charizard");





var Charizard = new pokemon("Charizard", 90, 30, 1, 15);
var Blastoise = new pokemon("Blastoise", 95, 28, 1, 14);
var Venasaur = new pokemon("Venasaur", 100, 26, 1, 13);

var poke_choice = new Array ("Charizard", "Blastoise", "Venasaur")



Now that we're done relying on these three pokemon variables to be in the global scope, we can clean up this part of the code, too.

We don't need to explicitly store the three pokemon variables anymore with their variable names; those can just go straight into the array. However, when we do that, we can no longer access the pokemon by name so it would be best to create a map/object for this situation:

var pokeChoice = {
    Charizard: new pokemon(...),
    Blastoise: new pokemon(...),
    ...
}


With this in place, this


for(i = 0; i&lt;= 2; i++) {
  if (poke_choice[i] === user.pokemon) {
    poke_choice.splice(i,1);
  }
}



can be simplified to:

if(pokeChoice.hasOwnProperty(user.pokemon.name)) {
    delete pokeChoice[user.pokemon.name];
}




Here is your full code now:

function pokemon(name, hp, critical_attack, lowest_attack, highest_attack) {
    this.name = name;
    this.hp = hp;
    this.critical_attack = critical_attack;
    this.lowest_attack = lowest_attack;
    this.highest_attack = highest_attack;   
}
var pokeChoice = {
    Charizard: new pokemon("Charizard", 90, 30, 1, 15),
    Blastoise: new pokemon("Blastoise", 95, 28, 1, 14),
    Venasaur: new pokemon("Venasaur", 100, 26, 1, 13)
};

function player(isComputer, name, pokemon) {
    this.isComputer = isComputer;
    this.name = name;
    this.pokemon = pokemon;
}
player.prototype.attack = function() {
    var uhigh = this.pokemon.highest_attack
    var ulow = this.pokemon.lowest_attack
    return Math.round(Math.random().toFixed(1)*uhigh)
}

var user = new player("User", "Arhum", pokeChoice.Charizard);
if(pokeChoice.hasOwnProperty(user.pokemon.name)) {
    delete pokeChoice[user.pokemon.name];
}

var comp_choice = function() {
    var pokeKeys = Object.keys(pokeChoice);
    var pokeAmt = pokeKeys.length;
    return pokeChoice[pokeKeys[Math.round(Math.random() * (pokeAmt-1))]]
}
var comp = new player("Computer","Red", comp_choice());    

console.log(user.attack())
console.log(comp.attack())


I made some other small changes here and there. If you have any questions, feel free to ask.

Also, the naming convection in JavaScript is:


PascalCase for classes.
camelCase for functions.


You are breaking these rules in some cases.