120
\$\begingroup\$

I recently started learning C++ about a week ago, and I made a bit of progress. In honor of Pokemon Go releasing, I decided to make a Pokedex with the original 151 Pokemon. So far I have 120 or so of them in there and it works PERFECTLY! (Thank goodness). I believe my code could be much more efficient and work better. I probably am using bad practices too. If I am, please feel free to tell me and if you find a problem, tell me!

Pokemon.h:

#pragma once
#include <string>


class Pokemon {
public:
    std::string type;
    double weight, height;
    std::string Gender;
    int evoLevel;
    bool finalEvo;
    int dexNum;
    std::string name;

    Pokemon(std::string name2, std::string type2, double weight2, double height2, std::string Gender2, int evoLevel2, bool finalEvo2, int dexNum2);
    Pokemon();
}; 

Pokemon.cpp:

#include "Pokemon.h"

Pokemon::Pokemon(std::string name2, std::string type2, double weight2,  double height2, std::string Gender2, int evoLevel2, bool finalEvo2, int dexNum2) {
    name = name2;
    type = type2;
    weight = weight2;
    height = height2;
    Gender = Gender2;
    evoLevel = evoLevel2;
    finalEvo = finalEvo2;
    dexNum = dexNum2;
}

//Default constructer
Pokemon::Pokemon() {
    name = "Pichario";
    type = "Death";
    weight = 10;
    height = 12;
    Gender = "Male and Female";
    evoLevel = 1;
    finalEvo = true;
    dexNum = 999;
}

main.cpp:

#include <iostream>
#include <string>
#include <vector>
#include "Pokemon.h"


int main() {
    //Create Pokemon objects
    Pokemon bulbasaur("Bulbasaur", "Grass and Poison", 15.2, 28, "Male and Female", 1, false, 1);
    Pokemon ivysaur("Ivysaur", "Grass and Poison", 28.7, 39, "Male and Female", 2, false, 2);
    Pokemon venusaur("Venusaur", "Grass and Poison", 220.5, 79, "Male and Female", 3, true, 3);

    Pokemon charmander("Charmander", "Fire", 18.7, 24, "Male and Female", 1, false, 4);
    Pokemon charmeleon("Charmeleon", "Fire", 41.9, 44, "Male and Female", 2, false, 5);
    Pokemon charizard("Charizard", "Fire and Flying", 199.5, 67, "Male and Female", 3, true, 6);

    Pokemon squirtle("Squirtle", "Water", 19.8, 20, "Male and Female", 1, false, 7);
    Pokemon wartortle("Wartortle", "Water", 49.6, 39, "Male and Female", 2, false, 8);
    Pokemon blastoise("Blastoise", "Water", 188.5, 63, "Male and Female", 3, true, 9);

    Pokemon caterpie("Caterpie", "Bug", 6.4, 12, "Male and Female", 1, false, 10);
    Pokemon metapod("Metapod", "Bug", 21.8, 28, "Male and Female", 2, false, 11);
    Pokemon butterfree("Butterfree", "Bug and Flying", 70.5, 43, "Male and Female", 3, true, 12);

    Pokemon weedle("Weedle", "Bug and Poison", 7.1, 12, "Male and Female", 1, false, 13);
    Pokemon kakuna("Kakuna", "Bug and Poison", 22, 24, "Male and Female", 2, false, 14);
    Pokemon beedrill("Beedrill", "Bug and Poison", 65, 39, "Male and Female", 3, true, 15);

    Pokemon pidgey("Pidgey", "Normal and Flying", 4, 12, "Male and Female", 1, false, 16);
    Pokemon pidgeotto("Pidgeotto", "Normal and Flying", 66.1, 43, "Male and Female", 2, false, 17);
    Pokemon pidgeot("Pidgeot", "Normal and Flying", 87.1, 59, "Male and Female", 3, true, 18);

    Pokemon rattata("Rattata", "Normal", 7.7, 12, "Male and Female", 1, false, 19);
    Pokemon raticate("Raticate", "Normal", 40.8, 28, "Male and Female", 2, true, 20);

    Pokemon spearow("Spearow", "Normal and Flying", 4.4, 12, "Male and Female", 1, false, 21);
    Pokemon fearow("Fearow", "Normal and Flying", 83.8, 47, "Male and Female", 2, true, 22);

    Pokemon ekans("Ekans", "Poison", 15.2, 79, "Male and Female", 1, false, 23);
    Pokemon arbok("Arbok", "Poison", 143.3, 138, "Male and Female", 2, true, 24);

    Pokemon pikachu("Pikachu", "Electric", 13.2, 16, "Male and Female", 1, false, 25);
    Pokemon raichu("Raichu", "Electric", 66.1, 31, "Male and Female", 2, true, 26);

    Pokemon sandshrew("Sandshrew", "Ground", 26.5, 24, "Male and Female", 1, false, 27);
    Pokemon sandslash("Sandslash", "Ground", 65, 39, "Male and Female", 2, true, 28);

    Pokemon nidoranf("Nidoran female", "Poison", 15.4, 16, "Female", 1, false, 29);
    Pokemon nidorina("Nidorina", "Poison", 44.1, 31, "Female", 2, false, 30);
    Pokemon nidoqueen("Nidoqueen", "Poison and Ground", 132.3, 51, "Female", 3, true, 31);

    Pokemon nidoranm("Nidoran Male", "Poison", 19.8, 20, "Male", 1, false, 32);
    Pokemon nidorino("Nidorino", "Poison", 43, 35, "Male", 2, false, 33);
    Pokemon nidoking("Nidoking", "Poison and Ground", 136.7, 55, "Male", 3, true, 34);

    Pokemon clefairy("Clefairy", "Fairy", 16.5, 24, "Male and Female", 1, false, 35);
    Pokemon clefable("Clefable", "Fairy", 88.2, 51, "Male and Female", 2, true, 36);

    Pokemon vulpix("Vulpix", "Fire", 21.8, 24, "Male and Female", 1, false, 37);
    Pokemon ninetales("Ninetales", "Fire", 43.9, 43, "Male and Female", 2, true, 38);

    Pokemon jigglypuff("Jigglypuff", "Normal and Fairy", 12.1, 20, "Male and Female", 1, false, 39);
    Pokemon wigglytuff("Wigglytuff", "Normal and Fairy", 26.5, 39, "Male and Female", 2, true, 40);

    Pokemon zubat("Zubat", "Poison and Flying", 16.5, 31, "Male and Female", 1, false, 41);
    Pokemon golbat("Golbat", "Poison and Flying", 121.3, 63, "Male and Female", 2, true, 42);

    Pokemon oddish("Oddish", "Grass and Poison", 11.9, 20, "Male and Female", 1, false, 43);
    Pokemon gloom("Gloom", "Grass and Poison", 19, 31, "Male and Female", 2, false, 44);
    Pokemon vileplume("Vileplume", "Grass and Poison", 41, 47, "Male and Female", 3, true, 45);

    Pokemon paras("Paras", "Bug and Grass", 11.9, 12, "Male and Female", 1, false, 46);
    Pokemon parasect("Parasect", "Bug and Grass", 65, 39, "Male and Female", 2, true, 47);

    Pokemon venonat("Venonat", "Bug and Poison", 66.1, 39, "Male and Female", 1, false, 48);
    Pokemon venomoth("Venomoth", "Bug and Poison", 27.6, 59, "Male and Female", 2, true, 49);

    Pokemon diglett("Diglett", "Ground", 1.8, 8, "Male and Female", 1, false, 50);
    Pokemon dugtrio("Dugtrio", "Ground", 73.4, 28, "Male and Female", 2, true, 51);

    Pokemon meowth("Meowth", "Normal", 9.3, 16, "Male and Female", 1, false, 52);
    Pokemon persian("Persian", "Normal", 70.5, 39, "Male and Female", 2, true, 53);

    Pokemon psyduck("Psyduck", "Water", 43.2, 31, "Male and Female", 1, false, 54);
    Pokemon golduck("Golduck", "Water", 168.9, 67, "Male and Female", 2, true, 55);

    Pokemon mankey("Mankey", "Fighting", 61.7, 20, "Male and Female", 1, false, 56);
    Pokemon primeape("Primeape", "Fighting", 70.5, 39, "Male and Female", 2, true, 57);

    Pokemon growlithe("Growlithe", "Fire", 41.9, 28, "Male and Female", 1, false, 58);
    Pokemon arcanine("Arcanine", "Fire", 341.7, 63, "Male and Female", 2, true, 59);

    Pokemon poliwag("Poliwag", "Water", 27.3, 24, "Male and Female", 1, false, 60);
    Pokemon poliwhirl("Poliwhirl", "Water", 44.1, 39, "Male and Female", 2, false, 61);
    Pokemon poliwrath("Poliwrath", "Water and Fighting", 119, 51, "Male and Female", 3, true, 62);

    Pokemon abra("Abra", "Psychic", 43, 35, "Male and Female", 1, false, 63);
    Pokemon kadabra("Kadabra", "Psychic", 124.6, 51, "Male and Female", 2, false, 64);
    Pokemon alakazam("Alakazam", "Psychic", 105.8, 59, "Male and Female", 3, true, 65);

    Pokemon machop("Machop", "Fighting", 43, 31, "Male and Female", 1, false, 66);
    Pokemon machoke("Machoke", "Fighting", 155.4, 59, "Male and Female", 2, false, 67);
    Pokemon machamp("Machamp", "Fighting", 286.6, 63, "Male and Female", 3, true, 68);

    Pokemon bellsprout("Bellsprout", "Grass and Poison", 8.8, 28, "Male and Female", 1, false, 69);
    Pokemon weepinbell("Weepinbell", "Grass and Poison", 14.1, 39, "Male and Female", 2, false, 70);
    Pokemon victreebel("Victreebel", "Grass and Poison", 34.2, 67, "Male and Female", 3, true, 71);

    Pokemon tentacool("Tentacool", "Water and Poison", 100.3, 35, "Male and Female", 1, false, 72);
    Pokemon tentacruel("Tentacruel", "Water and Poison", 121.3, 63, "Male and Female", 2, true, 73);

    Pokemon geodude("Geodude", "Rock and Ground", 44.1, 16, "Male and Female", 1, false, 74);
    Pokemon graveler("Graveler", "Rock and Ground", 231.5, 39, "Male and Female", 2, false, 75);
    Pokemon golem("Golem", "Rock and Ground", 661.4, 31, "Male and Female", 3, true, 76);

    Pokemon ponyta("Ponyta", "Fire", 66.1, 39, "Male and Female", 1, false, 77);
    Pokemon rapidash("Rapidash", "Fire", 209.4, 67, "Male and Female", 2, true, 78);

    Pokemon slowpoke("Slowpoke", "Water and Psychic", 79.4, 47, "Male and Female", 1, false, 79);
    Pokemon slowbro("Slowbro", "Water and Psychic", 173.1, 63, "Male and Female", 2, true, 80);

    Pokemon magnemite("Magnemite", "Electric and Steel", 13.2, 12, "Unknown", 1, false, 81);
    Pokemon magneton("Magneton", "Electric and Steel", 132.3, 39, "Unknown", 2, true, 82);

    Pokemon farfetchd("Farfetch'd", "Normal and Flying", 33.1, 31, "Male and Female", 1, true, 83);

    Pokemon doduo("Doduo", "Normal and Flying", 86.4, 45, "Male and Female", 1, false, 84);
    Pokemon dodrio("Dodrio", "Normal and Flying", 187.8, 71, "Male and Female", 2, true, 85);

    Pokemon seel("Seel", "Water", 198.4, 43, "Male and Female", 1, false, 86);
    Pokemon dewgong("Dewgong", "Water and Ice", 264.6, 67, "Male and Female", 2, true, 87);

    Pokemon grimer("Grimer", "Poison", 66.1, 35, "Male and Female", 1, false, 88);
    Pokemon muk("Muk", "Poison", 66.1, 47, "Male and Female", 2, true, 89);

    Pokemon shellder("Shellder", "Water", 8.8, 12, "Male and Female", 1, false, 90);
    Pokemon cloyster("Cloyster", "Water and Ice", 292.1, 59, "Male and Female", 2, true, 91);

    Pokemon gastly("Gastly", "Ghost and Poison", 0.2, 51, "Male and Female", 1, false, 92);
    Pokemon haunter("Haunter", "Ghost and Poison", 0.2, 63, "Male and Female", 2, false, 93);
    Pokemon gangar("Gengar", "Ghost and Poison", 89.3, 59, "Male and Female", 3, true, 94);

    Pokemon onix("Onix", "Rock and Ground", 463, 346, "Male and Female", 1, true, 95);

    Pokemon drowzee("Drowzee", "Psychic", 71.4, 39, "Male and Female", 1, false, 96);
    Pokemon hypno("Hypno", "Psychic", 166.7, 63, "Male and Female", 2, true, 97);

    Pokemon krabby("Krabby", "Water", 14.3, 16, "Male and Female", 1, false, 98);
    Pokemon kingler("Kingler", "Water", 132.3, 51, "Male and Female", 2, true, 99);

    Pokemon voltorb("Voltorb", "Electric", 22.9, 20, "Unknown", 1, false, 100);
    Pokemon electrode("Electrode", "Electric", 146.8, 47, "Male and Female", 2, true, 101);

    Pokemon exeggcute("Exeggcute", "Grass and Psychic", 5.5, 16, "Male and Female", 1, false, 102);
    Pokemon exeggutor("Exeggutor", "Grass and Psychic", 264.6, 79, "Male and Female", 2, true, 103);

    Pokemon cubone("Cubone", "Ground", 14.3, 16, "Male and Female", 1, false, 104);
    Pokemon marowak("Marowak", "Ground", 99.2, 39, "Male and Female", 2, true, 105);

    Pokemon hitmonlee("Hitmonlee", "Fighting", 109.8, 59, "Male", 1, true, 106);
    Pokemon hitmonchan("Hitmonchan", "Fighting", 110.7, 55, "Male", 1, true, 107);

    Pokemon lickitung("Lickitung", "Normal", 144.4, 47, "Male and Female", 1, true, 108);

    Pokemon koffing("Koffing", "Poison", 2.2, 24, "Male and Female", 1, false, 109);
    Pokemon weezing("Weezing", "Poison", 20.9, 47, "Male and Female", 2, true, 110);

    Pokemon rhyhorn("Rhyhorn", "Ground and Rock", 253.5, 39, "Male and Female", 1, false, 111);
    Pokemon rhydon("Rhydon", "Ground and Rock", 264.6, 75, "Male and Female", 2, true, 112);

    Pokemon chansey("Chansey", "Normal", 76.3, 43, "Female", 1, true, 113);

    Pokemon tangela("Tangela", "Grass", 77.2, 39, "Male and Female", 1, true, 114);

    Pokemon kangaskhan("Kangaskhan", "Normal", 176.4, 87, "Female", 1, true, 115);

    Pokemon horsea("Horsea", "Water", 17.6, 16, "Male and Gender", 1, false, 116);
    Pokemon seadra("Seadra", "Water", 55.1, 47, "Male and Female", 2, true, 117);

    Pokemon goldeen("Goldeen", "Water", 33.1, 24, "Male and Female", 1, false, 118);
    Pokemon seaking("Seaking", "Water", 86, 51, "Male and Female", 2, true, 119);

    Pokemon staryu("Staryu", "Water", 76.1, 31, "Unknown", 1, false, 120);
    Pokemon starmie("Starmie", "Water and Psychic", 176.4, 43, "Unknown", 2, true, 121);

    Pokemon mrmime("Mr. Mime", "Fairy and Psychic", 120, 51, "Male and Female", 1, true, 122);

    Pokemon scyther("Scyther", "Bug and Flying", 123.5, 59, "Male and Female", 1, true, 123);
    //Create a vector with all of the pokemon
    std::vector<Pokemon> pokemon = { bulbasaur, ivysaur, venusaur,
        charmander, charmeleon, charizard,
        squirtle, wartortle, blastoise,
        caterpie, metapod, butterfree,
        weedle, kakuna, beedrill,
        pidgey, pidgeotto, pidgeot,
        rattata, raticate,
        spearow, fearow,
        ekans, arbok,
        pikachu, raichu,
        sandshrew, sandslash,
        nidoranf, nidorina, nidoqueen,
        nidoranm, nidorino, nidoking,
        clefairy, clefable,
        vulpix, ninetales,
        jigglypuff, wigglytuff,
        zubat, golbat,
        oddish, gloom, vileplume,
        paras, parasect,
        meowth, persian,
        psyduck, golduck,
        mankey, primeape,
        growlithe, arcanine,
        poliwag, poliwhirl, poliwrath,
        abra, kadabra, alakazam,
        machop, machoke, machamp,
        bellsprout, weepinbell, victreebel,
        tentacool, tentacruel,
        geodude, graveler, golem,
        ponyta, rapidash,
        slowpoke, slowbro,
        magnemite, magneton,
        farfetchd,
        doduo, dodrio,
        seel, dewgong,
        grimer, muk,
        shellder, cloyster,
        gastly, haunter, gangar,
        onix,
        drowzee, hypno,
        krabby, kingler,
        voltorb, electrode,
        exeggcute, exeggutor,
        cubone, marowak,
        hitmonlee, hitmonchan,
        lickitung,
        koffing, weezing,
        rhyhorn, rhydon,
        chansey,
        tangela,
        kangaskhan,
        horsea, seadra,
        goldeen, seaking,
        staryu, starmie,
        mrmime,
        scyther };

    //Ask the user which way they would like to search
    std::cout << "Welcome to the Pokedex! We have the first " << pokemon.size() + 4 << " pokemon indexed!" << std::endl;
    std::cout << "Would you like to search by name, number, or type?" << std::endl;
    std::string input = "";
    std::cin >> input;
    //make input uppercase
    for (int p = 0; p < input.size(); p++) {
        input[p] = toupper(input[p]);
    }
    //check for input
    if (input == "NAME") {
        //Ask for name
        std::cout << "Enter the name of the pokemon" << std::endl;
        std::cin >> input;
        bool found = false;
        //make name lowercase
        for (int j = 0; j < input.length(); j++) {
            input[j] = tolower(input[j]);
        }
        //make first letter uppercase
        input[0] = toupper(input[0]);

        //loop through vector
        for (int i = 0; i < pokemon.size(); i++) {
            //check if input is the same as a pokemon name
            if (input == pokemon.at(i).name) {
                std::cout << "The pokemon " << pokemon.at(i).name << " has been found!" << std::endl;
                std::cout << pokemon.at(i).name << " is a " << pokemon.at(i).type << " type." << std::endl;
                std::cout << pokemon.at(i).name << "'s weight and height are: " << pokemon.at(i).weight << " lbs and " << pokemon.at(i).height << " inches." << std::endl;
                std::cout << pokemon.at(i).name << "'s gender is " << pokemon.at(i).Gender << std::endl;

                if (pokemon.at(i).finalEvo) {
                    std::cout << pokemon.at(i).name << " is the final evolution in its line." << std::endl;

                    if (pokemon.at(i).evoLevel > 1) {
                        std::cout << pokemon.at(i).name << " is what " << pokemon.at(i - 1).name << " evolves into." << std::endl;
                    } 
                    else {
                        std::cout << pokemon.at(i).name << " is the only pokemon in its evolution line" << std::endl;
                    }

                }
                else {
                    std::cout << pokemon.at(i).name << " evolves into " << pokemon.at(i + 1).name << std::endl;
                    if (pokemon.at(i).evoLevel > 1) {
                        std::cout << pokemon.at(i).name << " is what " << pokemon.at(i - 1).name << " evolves into." << std::endl;
                    }
                }
                found = true;
                break;
            }
        }
        //what to do if invalid pokemon is entered
        if (found == false) {
            std::cerr << "Pokemon not found!" << std::endl;
        }
    }
    else if (input == "TYPE") {
        //get input
        std::string input = "";
        std::cout << "Enter the type you would like!" << std::endl;
        std::cin >> input;
        //make input lowercase
        for (int k = 0; k < input.size(); k++) {
            input[k] = tolower(input[k]);
        }
        //make first letter uppercase
        input[0] = toupper(input[0]);

        //loop through vector
        for (int i = 0; i < pokemon.size(); i++) {
            //see if pokemon type contains the input
            if (pokemon.at(i).type.find(input) != std::string::npos) {
                std::cout << pokemon.at(i).name << " is a " << pokemon.at(i).type << " type!" << std::endl;
            }
        }
    }
    else {
        std::cout << "Enter the number of the pokemon" << std::endl;
        int input = 1;
        std::cin >> input;
        bool found = false;

        std::cout << input << std::endl;

        for (int i = 0; i < pokemon.size(); i++) {
            //see if pokemon has same num as input
            if (input == pokemon.at(i).dexNum) {
                std::cout << "The pokemon " << pokemon.at(i).name << " has been found!" << std::endl;
                std::cout << pokemon.at(i).name << " is a " << pokemon.at(i).type << " type." << std::endl;
                std::cout << pokemon.at(i).name << "'s weight and height are: " << pokemon.at(i).weight << " lbs and " << pokemon.at(i).height << " inches." << std::endl;
                std::cout << pokemon.at(i).name << "'s gender is " << pokemon.at(i).Gender << std::endl;

                if (pokemon.at(i).finalEvo) {
                    std::cout << pokemon.at(i).name << " is the final evolution in its line." << std::endl;
                    if (pokemon.at(i).evoLevel > 1) {
                        std::cout << pokemon.at(i).name << " is what " << pokemon.at(i - 1).name << " evolves into." << std::endl;
                    }
                    else {
                        std::cout << pokemon.at(i).name << " is the only pokemon in its evolution line" << std::endl;
                    }

                }
                else {
                    std::cout << pokemon.at(i).name << " evolves into " << pokemon.at(i + 1).name << std::endl;
                    if (pokemon.at(i).evoLevel > 1) {
                        std::cout << pokemon.at(i).name << " is what " << pokemon.at(i - 1).name << " evolves into." << std::endl;
                    }
                }
                found = true;
                break;
            }
        }
        //Say pokemon wasnt 
        if (found == false) {
            std::cerr << "Pokemon not found!" << std::endl;
        }

    }

    return 0;
}
\$\endgroup\$
9
  • 21
    \$\begingroup\$ "Pichario" isn't a real Pokemon. I would lose the default ctor. \$\endgroup\$ Commented Jul 20, 2016 at 1:20
  • 4
    \$\begingroup\$ Your finalEvo and evoLevel logic doesn't jive well with the Eeveelutions. Jolteon (#135) does not evolve into Flareon (#136), and Eevee (#133) is not restricted to evolving into Vaporeon (#134). \$\endgroup\$
    – Etheur
    Commented Jul 20, 2016 at 20:27
  • 13
    \$\begingroup\$ Am I the only one who sees questions like this and thinks that people should really be designing schemas in some database instead of classes in some OO language? If I were you, I'd be storing all this information in a database, and then I'd use some C++ SQL client to do stuff with that data. I realize that you're just learning C++, but this is why I think people should really learn SQL first. \$\endgroup\$ Commented Jul 21, 2016 at 1:30
  • 2
    \$\begingroup\$ rather than typing in an entire pokédex by hand, perhaps look for something that's already done the work for you :) github.com/veekun/pokedex \$\endgroup\$
    – Eevee
    Commented Jul 21, 2016 at 23:27
  • 2
    \$\begingroup\$ @michaelsnowden +1, though as a learning project and as it's essentially one big table he could do with a text file and read from it. \$\endgroup\$
    – Shautieh
    Commented Jul 23, 2016 at 12:59

5 Answers 5

155
\$\begingroup\$

Your Pokemon class (declaration)

class Pokemon {
public:
    std::string type;
    double weight, height;
    std::string Gender;
    int evoLevel;
    bool finalEvo;
    int dexNum;
    std::string name;

    Pokemon(std::string name2, std::string type2, double weight2, double height2, std::string Gender2, int evoLevel2, bool finalEvo2, int dexNum2);
    Pokemon();
}; 

Here are several things that spring to eye. First of all, all your fields are public. This indicates that you want a struct, not a class. In C++, they only differ by the default visibility. class is private by default, struct is public by default. So let's change that:

struct Pokemon {

Next, we should disable Pokemon(). That way, one cannot create a pokemon by accident:

    Pokemon() = delete;

Note that this isn't necessary, since any constructor will prevent the compiler from providing a default one.

Better types

In your other constructor, we change the name of the arguments and their types slightly:

    Pokemon(Name name, Type type, Weight weight, Height height, Gender gender, 
            EvoLevel level, bool finalEvolution, PokedexID pid);

Wait. What the hell are all those types? First of all, they are an overkill. Second of all, they follow the C++ core guidelines. You don't want to use a Pokemon's name for its gender by accident, do you?

Handling the Pokemon's type

Which brings us to enumerations. Type and Gender are perfect candidates for those:

enum class BasicType {
   Fire,
   Grass,
   Water,
   Electro,
   Poison,
   ...
};

Since Pokemon can have several types, we use another type Type to combine them:

class Type{ 
    Type(BasicType main);
    Type(BasicType main, BasicType sub);

    void addSubType(BasicType);
    bool hasType(BasicType) const;
};

I'm not sure whether Pokemon will ever have more than two types, but as @Eeevee noticed, they are ordered.

If you want, you can define syntactic sugar upon this with by overloading operator|:

Type operator|(BasicType main, BasicType sub) { return Type(main, sub); }
Type operator|(Type a,         BasicType sub) { return a.addSubtype(sub); }

The operator enables us to use Type::Fire | Type::Poison. Note that this is probably again an overkill. However, it will prevent you from using

Pokemon("Bulbasaur", "Gras and Poison", ...);

Instead, if you use

Pokemon("Bulbasaur", Type::Gras | Type::Poison, ...);

you end up with a compiler error, instead of a frustrated user not able to find "Bulbasaur" by type "Grass". For testing, an operator& might come in handy:

Type operator&(Type      a, BasicType b) { return a.hasType(b); }
Type operator&(BasicType a, Type      b) { return b.hasType(a); }

That's very basic, but it gets its job done. How you actually store the types is left as an exercise, but not too hard. Note that you can add arbitrary methods to Type or functions that take Type as an argument, so there is enough space for additional mad science experiments features.

A gender study

We can do the same for gender, although it's slightly easier here, since there are only some and therefore we don't need the operator| trick or addSubType:

enum class Gender {
   Male,
   Female,
   Both,
   Unknown
};

Again, this prevents you from writing

Pokemon("Bulbasaur", Type::Grass | Type::Poison, 15.2, 28, "Apache helicopter", ...);

Putting it all together

For the other types, for the sake of simplicity, we say

typedef std::string Name;
typedef double Weight;
typedef double Height;
typedef unsigned int EvoLevel;
typedef unsigned int PokedexID;

Therefore, we end up with the following struct:

struct Pokemon {
    Pokemon() = delete;
    Pokemon(Name name, Type type, Weight weight, Height height, Gender gender, 
            EvoLevel level, bool isFinalEvolution, PokedexID pid);

  Name name;
    Type type;
    Weight weight;
    Height height;
    Gender gender;
    EvoLevel evolutionLevel;
    bool isFinalEvolution;
    PokedexID pid;
};

Note that contrary to your code, all variables here strictly follow camelCase, whereas your code contained Gender. Also notice that in class objects the members are usually prefixed with m_, suffixed with _ or follow another naming convention. Since you want to use it without encapsulation and all public (e.g. pokemon.type) prefixes or suffixes would make your code harder to read here.

"But wait,", I here you say. "Now the arguments have the same name as my members!". And that's good. If someone uses Pokemon::Pokemon(...) in their program, they want their IDE to show the argument names in a nice way, without the 2 as suffix.

Which brings us to your constructor's implementation:

// your code
#include "Pokemon.h"

Pokemon::Pokemon(std::string name2, std::string type2, double weight2,  double height2, std::string Gender2, int evoLevel2, bool finalEvo2, int dexNum2) {
    name = name2;
    type = type2;
    weight = weight2;
    height = height2;
    Gender = Gender2;
    evoLevel = evoLevel2;
    finalEvo = finalEvo2;
    dexNum = dexNum2;
}

Aha. First of all, if you have an argument that has the same name as a member, you can use this->[membername] to distinguish, e.g.

this->name = name2;

However, we're in a constructor. Therefore, we can initialize our values. Note that this isn't necessary with a POD, but some of your members are classes, for example Name (it's a std::string). This saves a copy. We end up with:

Pokemon(Name name, Type type, Weight weight, Height height, Gender gender, 
        EvoLevel level, bool isFinalEvolution, PokedexID pid) 
 : name(std::move(name)), type(type), weight(weight), height(height), gender(gender),
   evolutionLevel(evolutionLevel), isFinalEvolution(isFinalEvolution), pid(pid) {
}

Note that this indicates that we didn't really need have defined a constructor, we could just have used our usual struct initialisation syntax

Pokemon bulbasaur = { "Bulbasaur", ... };

But depending on your types, that's slightly more error prone.

Output and DRY

You don't follow the Don't-Repeat-Yourself principle. Your code contains the facilities to print a Pokemon twice. This is bad, since changes on one part should probably also be done on the other, but get lost. Instead, provide a operator, if you want to use std::cout:

std::ostream & operator<<(std::ostream & out, const Pokemon & pokemon) {
    out << pokemon.name << " is a " << pokemon.type << " type.\n"
        << pokemon.name << "'s weight and height are: " 
        << pokemon.weight << " lbs and " << pokemon.height << " inches.\n";
        << pokemon.name << "'s gender is " << pokemon.gender << "\n";

    if (pokemon.isFinalEvolution) {
        out << pokemon.name << " is the final evolution in its line.\n";
    }
    ...
    return out;
}

Note that this also needs operator<< on all types used in Pokemon, e.g. Gender and Type.

The database

So here's my greatest pet peeve with your code. Your main:

Pokemon bulbasaur("Bulbasaur", "Grass and Poison", 15.2, 28, "Male and Female", 1, false, 1);
Pokemon ivysaur("Ivysaur", "Grass and Poison", 28.7, 39, "Male and Female", 2, false, 2);
Pokemon venusaur("Venusaur", "Grass and Poison", 220.5, 79, "Male and Female", 3, true, 3);
...

There are two things I'm not fond of here. First of all, you only use the names "bulbasaur" or "ivysaur" once later, in std::vector<Pokemon> pokemon, so you're polluting the local namespace. Instead, fill your vector directly:

std::vector<Pokemon> pokedex = { 
    Pokemon("Bulbasaur", Type::Grass | Type::Poison, 15.2, 28, Gender::Both, 1, false, 1),
    Pokemon("Ivysaur", Type::Grass | Type::Poison, 28.7, 39, Gender::Both, 2, false, 2),
    ...
};

But that's not how you should do it.

So far I have 120 or so of them in there and it works PERFECTLY!

You're entering all of them by hand. That's insane. Let's say you've messed the lbs up of Pikachu, or you want to add more Pokemon later. You will always have to edit your program and compile it. That's bad. It's error prone. And the errors are hard (compiling) to fix.

Instead, have all your Pokemon in an external file. You can then read from that file into your vector easily (if you write the streaming operator):

while(pokeFile >> pokemon){
    pokedex.push_back(pokemon);
}

That's all that's necessary, apart from getting the Pokemon data into the external file and writing istream& operator>>(istream&, Pokemon&). Instead of a main that gets larger with every other Pokemon generation, you only have to change your Pokemon.txt file. You can even translate it to other languages (e.g. "Bisasam", "Bisaknosp", "Bisaflor",...), but that's a little bit far at this point.

Other stuff

If you don't need the index, use a range-based for loop instead of operator[]. Also, you've shadowed the variable input several times. Don't. Furthermore strive for smaller functions, your main was too large.

If you're using C++11, use the range-based for loop more often.

Also if you don't want to modify a variable, use const. Your std::vector<Pokemon> is a perfect candidate for this, since you don't want to change it afterwards.

Concerning ostream& operator<<(ostream&,const Pokemon&), we've lost the ability to print the previous/next evolution, but that's by design, since you need access on the whole Pokedex for that (and there are possibly several evoltions for a single Pokemon, looking at you, Eevee). That's something you should probably think about a little bit more. You could store the ID of the previous Pokemon instead, or use an invalid ID if it does not exist. But that's left as an exercise.

Last but not least, if you were to write a productive Pokedex, use libraries that enable you to use database(-like) functions, for example SQLite, and/or abstract the queries behind a class, like

class Pokedex {
public:
    template<class F>
    Result find_one(F && f){
        return from_it(std::find(internal_list.begin(), internal_list.end(), f));
    }
    ResultList find_all(F && f){
        ResultList results;
        std::copy_if(internal_list.begin(), internal_list.end(), ...);
        return results;
    }
};

depending how complex your queries should be.

Your "new" main

So, following the remarks above, I've somewhat rewritten your main. The Pokemon are gone from the cage of your code, instead they roam in some other Pokeball file.

#include <iostream>
#include <string>
#include <vector>
#include "Pokemon.h"

typedef std::vector<Pokemon> Pokedex;

std::string to_pokecase(const std::string & str){
    // excercise, should make "Bulbasaur" from "buLbaSAUR".
}

bool print_pokemon_by_name(const Pokedex & pokedex){
    //Ask for name
    std::cout << "Enter the name of the pokemon" << std::endl;
    Name input_name;
    std::cin >> input_name;

    input_name = to_pokecase(input_name);

    for (auto & thePokemon : pokedex) {
        if (input_name == thePokemon.name) {
            std::cout << "The pokemon " << thePokemon.name << " has been found!\n" 
                      << thePokemon<< std::endl;
            return true;
        }
    }

    std::cerr << "Pokemon not found!" << std::endl;
    return false;
}

bool print_pokemons_by_type(const Pokedex & pokedex){
    // exercise
}

bool print_pokemon_by_number(const Pokedex & pokedex){
    // exercise
}

std::vector<Pokemon> read_pokemons_from_file(const std::string & fileName){
    // exercise
}

int main() {
    const auto pokedex = read_pokemons_from_file("Pokemon.txt");

    //Ask the user which way they would like to search
    std::cout << "Welcome to the Pokedex! We have the first " << pokedex.size() + 4 << " pokemon indexed!" << std::endl;
    std::cout << "Would you like to search by name, number, or type?" << std::endl;
    std::string input = "";
    std::cin >> input;

    //make input uppercase
    for (auto & p : input) {
        p = toupper(p);
    }

    if (input == "NAME") {
        print_pokemon_by_name(pokedex);
    }
    else if (input == "TYPE") {
        print_pokemons_by_type(pokedex);
    }
    else if (input == "NUMBER") {
        print_pokemon_by_number(pokedex);    
    } else {
        // handle invalid input
    }    
    return 0;
}

As you can see, the new main is much smaller. It can be read on a screen without scrolling at all. All functionalities that differ have been extracted into other functions, which themselves are rather small again.

There are several things that can be further refactored and rewritten, but that's left as an exercise.

Addendum: The specification

Thanks to all comments on this review and all the Pokemon specification I've missed in several revisions of this review, I guess this is a good place to add two things.

First of all, before you implement your class, ask yourself more questions: Can you easily test for (pokemon) types? Do you often test for them? Should that be in its own function? Do you test for names? Do you need to normalize names? More than once? Do the Pokemon's types need to be ordered?

Secondly, reflect the answers to those questions in your code, or, if the answer was negative, in your comments/documentation. Also, try to use higher-level code. The main above is already pretty high-level, with the effect that I can change Pokedex and Pokemon completely and never need to touch main at all.

Either way, good luck catching 'em all! May your Pokedex be complete.

\$\endgroup\$
22
  • 1
    \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$
    – Jamal
    Commented Jul 21, 2016 at 4:45
  • 2
    \$\begingroup\$ ´Pokemon() = delete;´ is not needed at all. Generally functions with more then 2 parameters are frowned upon. I would remove all constructors. \$\endgroup\$ Commented Jul 21, 2016 at 12:15
  • 2
    \$\begingroup\$ a set is not an appropriate data type for a pokémon's type — the types are ordered. the order doesn't affect game mechanics, but it's always preserved in all official sources and has some flavor/lore impact. \$\endgroup\$
    – Eevee
    Commented Jul 21, 2016 at 23:26
  • 1
    \$\begingroup\$ Remove Pokemon(Name name, Type type, Weight weight, Height height, Gender gender, EvoLevel level, bool isFinalEvolution, PokedexID pid) ctor and use {}. (Try it before you bash it) \$\endgroup\$
    – Yakk
    Commented Jul 22, 2016 at 21:34
  • 4
    \$\begingroup\$ Great post, but I disagree with "Don't use std::vector<T>::at(i)". There's about a billion exploits and hard to find bugs in C++ code due to no bounds checking with []. Yes there's some marginal overhead that can be elided most of the time by the compiler, but this won't matter at all here. \$\endgroup\$
    – Voo
    Commented Jul 23, 2016 at 9:58
35
\$\begingroup\$

Just a few things that seem like they might be open to improvement.

Evolution

Rather than having a Boolean to indicate whether a particular type is the final step in its evolution, I think I'd have an "evolvesTo" field to give the dexNum of the type to which this evolves, and (probably) use a special value such as -1 to indicate that it doesn't evolve into anything else. I'd probably also add a similar field for "evolvesFrom". At least in my opinion, encoding the information directly is much cleaner than depending on the order of records.

Database

Rather than having all that raw data embedded into the program, I'd at least consider moving it into an external file, which you'd then read in when you start the program. Doesn't have to be anything complex or special--for example, I'd consider something like one Pokemon per line, with a tab between each field.

Constructor

It's generally preferable to initialize members with a member initializer list rather than assign to them in the body of a constructor. When you do this, the compiler can sort out which is the parameter and which is the member, so you don't need to add a 2 to each parameter name to keep them straight:

Pokemon::Pokemon(std::string name, std::string type, double weight,  double height, std::string Gender, int evoLevel, bool finalEvo, int dexNum) 
    : name(name),
      type(type),
      weight(weight),
      height(height),
      Gender(Gender),
      evoLevel(evoLevel),
      finalEvo(finalEvo),
      dexNum(dexNum),
{}

Searching

You have quite a few instances of searching through your database to find Pokemon based on various attributes. Nearly all these can be implemented with std::find_if to do the actual searching.

auto p = std::find_if(pokemon.begin(), pokemon.end(),
    [&](int dexNum) { return dexNum == input; });

if (p == pokemon.end())
    std::cout << "Sorry, I can't find that Pokemon.\n";
else {
    std::cout << "The Pokemon: " << p->name << " has been found.\n";
    std::cout << p->name << " is a " << p->type << " type Pokemon\n";
    // ...
}

If you were dealing with a lot more data (tens of thousands or tens of millions of items) there are more efficient ways of searching data, such as building an index for each field you want to search by. If you want to experiment with one implementation of this, the Boost Multi Index library provides this type of functionality.

On the other hand, if you have to deal with that amount of data, you probably want to use some sort of database anyway (e.g., SQLite).

\$\endgroup\$
5
  • \$\begingroup\$ +1 for the database suggestion. Also, table design: tables for the pokemon themselves, type, gender, and evolution path. More at <en.wikipedia.org/wiki/Entity%E2%80%93relationship_model> <en.wikipedia.org/wiki/Class_diagram> <en.wikipedia.org/wiki/Database_design> \$\endgroup\$ Commented Jul 19, 2016 at 23:24
  • 1
    \$\begingroup\$ The evolution suggestion is good, but there are several possible evolutions. Take Eevee for example. It can evolve into Vaporeon, Jolteon, Flareon, Espeon, and several others, so evolveTo has to be a set or something similar. On the other hand evolveFrom would be either one or none. \$\endgroup\$
    – Zeta
    Commented Jul 20, 2016 at 5:36
  • \$\begingroup\$ Seems overkill to capture everything by reference in your lambda, when you only need to capture input. \$\endgroup\$ Commented Jul 20, 2016 at 13:33
  • 1
    \$\begingroup\$ @cloakedlearning: even with a default capture, a lambda still only captures what's used in the lambda. \$\endgroup\$ Commented Jul 20, 2016 at 13:46
  • 2
    \$\begingroup\$ @cloakedlearning: If you want the gory details, they're in [expr.prim.lambda]/12 of the standard/draft standard. Most of the relevant part is: "A lambda-expression with an associated capture-default [...] is said to implicitly capture the entity (i.e., this or a variable) if the compound-statement: odr-uses the entity, or names the entity in a potentially-evaluated expression [...]". \$\endgroup\$ Commented Jul 20, 2016 at 15:33
12
\$\begingroup\$

The biggest problem I see with your code is Eevee.

Eevee has a branched evolution.

My fix is to have a vector with the dex number of the evolutions (And if you wanted to get fancy it would be a struct with the details of the evolution. (Type,Level and dex number )

\$\endgroup\$
6
  • \$\begingroup\$ And additional tools needed for the evolution, e.g. water stone. \$\endgroup\$
    – Zeta
    Commented Jul 20, 2016 at 5:43
  • \$\begingroup\$ This also becomes important if OP intends to go beyond the original 151, since Gen 2 and onward started introducing out-of-order evolutionary lines. \$\endgroup\$
    – BJ Myers
    Commented Jul 20, 2016 at 21:17
  • \$\begingroup\$ I did think about Eeveelutions until I made this question... lol :P. Right now, I have it hard coded into the code. It is saying that if it's Jolteon, Vaporeon, etc. It says [pokemon] is what Eevee evolves into. Very inefficient and messy but it works :P. \$\endgroup\$ Commented Jul 21, 2016 at 0:06
  • \$\begingroup\$ Eevee is not the only branched evolution. As far back as generation II, pokemon such as Gloom, Polywhirl and Slowpoke were granted branched evolutions. \$\endgroup\$
    – Pharap
    Commented Jul 21, 2016 at 4:31
  • \$\begingroup\$ Better solution would be an int evolvesFrom field guaranteeing one to many relationship. It would make searching for final forms a bit more complex, but that can be done once per program run and the index is just a simple integer so it's just a blink of an eye. \$\endgroup\$
    – loa_in_
    Commented Jul 21, 2016 at 13:03
8
\$\begingroup\$

So you wanna be a master of Pokemon? Do you have the skills to make a Pokedex?

The Pokedex is a piece of complicated machinery. As a result, this code review will be more focused on the conceptual approach; how your data relates to other data and the best ways to represent that data. It will not cover implementation details, such as whether the actual data is derived from text files, databases, is hardcoded, etc. Nor will it cover how best to organize the code in different files. So... here we go...

There are quite a lot of things to think about when creating a Pokedex, even one as simple as what you've got going on. So first we'll take a look at design decisions.

1: Types

There are (currently) eighteen types that a Pokemon can be. These types are well-defined and discrete; that is to say, there are no gradients between types and the set of available types do not change dynamically. As a result, types are best represented as an enum.

enum class Type {
  Fire,
  Water,
  Grass,
  Electric,
  Fighting,
  Flying,
  Poison,
  Ground,
  Ice,
  Psychic,
  Ghost,
  Dragon,
  Fairy,
  Steel,
  Dark,
  Rock,
  Normal,
  Bug
};

There are also no correlations between types and integral values, so the order in the enum does not matter.

In addition, you will want a function somewhere that takes a type and returns a string, being human-readable.

std::string TypeToStr(Type type) {
  switch(type) {
  case Type::Fire:
    return "Fire";
  case Type::Water:
    return "Water";
  ...
  default:
    return "ERROR"; //should never get here if you accounted for all types
}

2: Items

We're gonna need this later. Trust me

enum class Item {
  WaterStone,
  FireStone,
  Thunderstone,
  MoonStone,
  LeafStone
};

And a method to get the human-readable name

std::string ItemToStr(Item item) {
  switch(item) {
  case Item::WaterStone:
    return "Water Stone";
  case Item::FireStone:
    return "Fire Stone";
  case Item::Thunderstone:
    return "Thunderstone";
  ...
  default:
    return "ERROR"; //Should not get here if you accounted for all items
  }
}

3: Evolution

Most people know Pokemon can evolve. Many of these evolutions are linear (a Krabby can evolve into only a Kingler). But some have branched evolutions. Eevee, in Generation I, can evolve into Jolteon, Flareon or Vaporeon. So what to do? We could store the three pieces of evolution data three times for each Pokemon, even though Eevee is the only one in Gen I that will use more than one. I consider that to be wasteful. We could store evolution data in a vector. That's better, but there's a clever and simple solution: go backwards.

No Pokemon can be evolved to from more than one base form. Let me repeat that: There are no two Pokemon that evolve into the same thing. So we can store our evolution data one per Pokemon and instead of the evolution data saying "This is what you need to do to get to the next stage and this is what the next stage is" it will say "This is how the previous stage got to this form and this is what the previous stage was".

So what does the evolution data look like? Well, in Generation I there are three ways a Pokemon can evolve: Level up (at a certain level), Trade and Stone (with a certain stone). So we have method, data and form. This is what it may look like in code:

class Evo {
public:
  Evo(int from) :
    fromID(from)
  {}

  virtual std::string ToStr() = 0;

  int fromID;
};

class EvoTrade : public Evo {
public:
  EvoTrade(int from) :
    Evo(from)
  {}

  virtual std::string ToStr() {
    return " by trading.";
  }
};

class EvoLevel : public Evo {
public:
  EvoLevel(int from, int lvl) :
    Evo(from),
    level(lvl)
  {}

  virtual std::string ToStr() {
    return " by reaching level " + std::to_string(level) + ".";
  }

  int level;
};

class EvoStone : public Evo {
public:
  EvoStone(int from, Item evo_stone) :
    Evo(from),
    stone(evo_stone)
  {}

  virtual std::string ToStr() {
    return " by using a " + ItemToStr(stone) + ".";
  }

  Item stone;
};

5: Genders

Pokemon have different chances of being male or female. There are also some that are genderless (many legendaries and also Ditto, and some whose design just calls for gender-agnosticism).

6: Pokemon

Finally we can get here. Okay.

Pokemon can have one or two types (in the dex; in battle, there are cases where they can have three). There are a couple ways to handle this. You could contain a single Type variable and essentially use a bitfield as has been mentioned before, but since a Pokemon can only have up to two types, I think this is overkill. Instead, I would recommend simply having two Type variables. Of course, this presents an extra conundrum.

If we have our Pokemon contain two Type variables, what do we do if the Pokemon has only one type? Say it's a Voltorb, which as you know is Electric only. The second Type variable must contain a valid value, so what do we do? We could introduce a nineteenth type in our enum that serves to mark a type as None, but there is another way. The way the games do it (last I checked) internally is set both variables to the same type. So that is the approach we will follow.

class Pokemon {
public:
  std::string name;
  double height, weight; //Metric
  double gender_ratio; //Chance of Male, -1 means genderless
  Evo* evo_data;
  int dex_num;
  Type type_1;
  Type type_2;
  std::function<std::vector<std::string>(int)> get_next_stages;

  Pokemon(int dex_number, const std::string& new_name, double new_height, double new_weight, double male_chance) :
    name(new_name),
    height(new_height),
    weight(new_weight),
    gender_ratio(male_chance),
    dex_num(dex_number),
    evo_data(nullptr)
  {}
  ~Pokemon() {
    delete evo_data;
  }

  std::string ToStr() {
    auto evolves_to = get_next_stages(dex_num);
    std::string answer = 
      "Number: " + std::to_string(dex_num) +
      "\nName: " + name;

      if(type_1 == type_2)
        answer += "\nType: " + TypeToStr(type_1);
      else
        answer += "\nTypes: " + TypeToStr(type_1) + '/' + TypeToStr(type_2);

      answer += "\nHeight: " + std::to_string(height) + 'm' +
      "\nWeight: " + std::to_string(weight) + "kg" +
      "\nMale/Female: ";

      if(gender_ratio < 0)
      {
        answer += "Genderless";
      }
      else
      {
        answer += std::to_string(gender_ratio) + '/';
        answer += std::to_string(1 - gender_ratio);
      }

      for(auto str : evolves_to)
        answer += '\n' + str;

      return answer;
  }

  //These next methods are used to chain setups; they are much more
  //flexible than defining constructors for every possible
  //combination of data setup. And we need to go through methods
  //to set this data up (at least, some of the data) because there
  //are some things we want to do with them

  Pokemon& set_types(Type type) {
    type_1 = type_2 = type;
    return *this;
  }
  Pokemon& set_types(Type type_a, Type type_b) {
    type_1 = type_a;
    type_2 = type_b;
    return *this;
  }

  Pokemon& set_evo_data(Evo* evo) {
    evo_data = evo;
    return *this;
  }
};

7: Final notes

So what does method chaining look like? Consider the Pokemon class above.

Pokemon p1(1, "Bulbasaur", 0.7, 6.9, 87.5)
  .set_types(Type::Grass, Type::Poison);
Pokemon p2(5, "Charmeleon", 1.1, 19, 87.5)
  .set_types(Type::Fire)
  .set_evo_data(new EvoLevel(4, 16));

And what about printing that data?

//Assuming a collection of Pokemon
std::vector<Pokemon> mons;
//Fill out the data
//...

auto next_stages = [&mons](const int id){
  std::vector<std::string> answer;

  auto found = std::find_if(mons.begin(), mons.end(), [id](const Pokemon& mon){
    return mon.evo_data != nullptr && mon.evo_data->fromID == id;
  });

  if(found != mons.end())
  {
    answer.push_back("Evolves into " + found->name + found->evo_data->toStr());
  }

  return answer;
};

//While filling out the data, put this function as the get_next_stages function
\$\endgroup\$
4
  • 1
    \$\begingroup\$ To get things' names, why use a massive switch statement when you could instead have a static array of std::string? Much less boilerplate and computation, ~same storage size. Just ensure the enum values start at 0 and don't contain gaps (if either of these aren't true, that seems like a code smell), then define the elements of the array in the same order (copy/paste the enum names as comments to make this easier/self-documenting), then use the enum to index into it. \$\endgroup\$ Commented Jul 24, 2016 at 8:54
  • \$\begingroup\$ That's definitely a different way of doing it. With only eighteen types and five items that matter in the context of this project for Gen I, I don't see a big difference. On a similar note, should op wish to expand this to other languages or support future generations with their trading with items, I would recommend storing that data (at least the items) in an outside source and read it in that way. \$\endgroup\$
    – Altainia
    Commented Jul 24, 2016 at 13:44
  • 1
    \$\begingroup\$ @underscore_d Your solution might be slightly shorter than using a switch, but it's also more fragile. What happens when someone reorders the enum values, or adds/removes one in the middle, but doesn't change the name array? Oops, half the stuff now has the wrong name. \$\endgroup\$
    – bcrist
    Commented Jul 24, 2016 at 16:47
  • \$\begingroup\$ @bcrist True and worth noting; +1. But I get good mileage out of this method in several projects. It just requires an awareness of one's situation and the fact that changes need to be made in 2 places. Plus, I tend to define the enums once and never need to extend them. I guess my cases are simplistic... In a project with multiple committers, who aren't guaranteed to be keenly aware of this constraint, and where new values may arise - then sure, use the switch-based method to be safe. Or perhaps, for code that looks nicer but is necessarily slower, some kind of map with an enum as key. \$\endgroup\$ Commented Jul 24, 2016 at 16:53
4
\$\begingroup\$

V nice code. Plus points for

  • not doing using namespace std
  • using standard containers

Comments

Be consistent with naming things, members of Pokemon class type, Gender.

I would strongly suggest using a special naming convention for member variables. Some dont like this , but I do. so use m_gender or gender_, this makes code clearer in functions, you know what data you are manipulating

I dont think you should have a default constructor. Its meaningless and not needed

Break the code up into functional chunks, LoadPokemons, PrintPoke, PromptUser, ....

Use map not vector. This allows direct lookup rather than linear searching

Notice that you repeat the code for string upper-> lower and vice versa. Make functions to do this.

Future idea. Load the pokemon data from a file rather than compiling into the program.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ map only allows searching on one field though--he supports searching on three (and he's never going to have tens of thousands of Pokemon to search through, or anything like that). \$\endgroup\$ Commented Jul 19, 2016 at 17:56
  • 1
    \$\begingroup\$ @JerryCoffin considering that we've seen pretty consistent additions of pokemon I wouldn't speak too soon \$\endgroup\$ Commented Jul 19, 2016 at 18:00
  • 1
    \$\begingroup\$ @Dannnno: In ~20 years, they've gotten to just over 700 Pokemon. At the rate they're going so far, it'll be ~275 years to get to ten thousand Pokemon. \$\endgroup\$ Commented Jul 19, 2016 at 18:22
  • \$\begingroup\$ I think learning to use map is a useful excercise, I must admit I had not seen the multi key lookup, so OP should use index maps that map keyn to primary key. \$\endgroup\$
    – pm100
    Commented Jul 19, 2016 at 18:23
  • \$\begingroup\$ @JerryCoffin I wouldn't be that surprised if it lasts another 275 years </facetious> \$\endgroup\$ Commented Jul 19, 2016 at 18:25

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