Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Our instructor told us to create a basic Java program using everything we've learned in class thus far (basic prints, selection structures, looping, GUI, arrays, etc) and being a Pokemon fan, I decided to make a basic GUI Pokedex that lets the user search for the details of a particular starter Pokemon.

This program also asks the user if he wants to search by Pokemon type or by region of origin, just in case the user does not know the name of the Pokemon.

Here is the code I came up with and it runs:

import javax.swing.*;
public class StarterPokedex {

    public static void main(String[] args) {

        String choice, desc=null, pokemon=null, searchBy=null, region=null, type=null, pokeNo=null, species=null, habitat=null; 

        choice = JOptionPane.showInputDialog(null, "Do you know the name of the Starter Pokemon you are looking for? (Yes/No)", "Welcome to the Starter Pokedex!", 3); 
        if (choice.equalsIgnoreCase("yes")) {
            pokemon = JOptionPane.showInputDialog(null, "Enter the name of the Starter Pokemon: ", "Welcome to the Starter Pokedex!", JOptionPane.PLAIN_MESSAGE); }
        else if (choice.equalsIgnoreCase("no")) {
            searchBy = JOptionPane.showInputDialog(null, "Search by: 'type' or 'region'?", "Welcome to the Starter Pokedex!", 3);

                if (searchBy.equalsIgnoreCase("type")) {
                    type = JOptionPane.showInputDialog(null, "Enter pokemon type: (Fire/Water/Grass)", "Welcome to the Starter Pokedex!", JOptionPane.PLAIN_MESSAGE);

                    if (type.equalsIgnoreCase("Fire")) {
                    pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are fire-type: \n\nCharmander \nCyndaquil \nTorchic \nChimchar \nTepig \nFennekin \n\nWhich Pokemon would you like to search?", "Fire-type Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (type.equalsIgnoreCase("Water")) {
                        pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are water-type: \n\nSquirtle \nTotodile \nMudkip \nPiplup \nOshawott \nFroakie \n\nWhich Pokemon would you like to search?", "Water-type Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (type.equalsIgnoreCase("Grass")) {
                        pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are grass-type: \n\nBulbasaur \nChikorita \nTreecko \nTurtwig \nSnivy \nChespin \n\nWhich Pokemon would you like to search?", "Grass-type Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }

                        else {
                        JOptionPane.showMessageDialog(null, "You entered an invalid keyword. Try again.", "Error!", 1); }
                    }


                else if (searchBy.equalsIgnoreCase("region")) {
                    region = JOptionPane.showInputDialog(null, "Enter region: (Kanto/Johto/Hoenn/Sinnoh/Unova/Kalos)", "Welcome to the Starter Pokedex!", JOptionPane.PLAIN_MESSAGE); 

                    if (region.equalsIgnoreCase("Kanto")) {
                    pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are from the Kanto region: \n\nBulbasaur \nCharmander \nSquirtle \n\nWhich Pokemon would you like to search?", "Kanto Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (region.equalsIgnoreCase("Johto")) {
                            pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are from the Johto region: \n\nChikorita \nCyndaquil \nTotodile \n\nWhich Pokemon would you like to search?", "Johto Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (region.equalsIgnoreCase("Hoenn")) {
                            pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are from the Hoenn region: \n\nTreecko \nTorchic \nMudkip \n\nWhich Pokemon would you like to search?", "Hoenn Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (region.equalsIgnoreCase("Sinnoh")) {
                            pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are from the Sinnoh region: \n\nTurtwig \nChimchar \nPiplup \n\nWhich Pokemon would you like to search?", "Sinnoh Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (region.equalsIgnoreCase("Unova")) {
                            pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are from the Unova region: \n\nSnivy \nTepig \nOshawott \n\nWhich Pokemon would you like to search?", "Unova Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }
                        else if (region.equalsIgnoreCase("Kalos")) {
                            pokemon = JOptionPane.showInputDialog(null, "Here are the list of Starter Pokemon that are from the Kalos region: \n\nChespin \nFennekin \nFroakie \n\nWhich Pokemon would you like to search?", "Kalos Starter Pokemon", JOptionPane.PLAIN_MESSAGE); }

                        else {
                        JOptionPane.showMessageDialog(null, "You entered an invalid keyword. Try again.", "Error!", 1); }

                }

            else {
                JOptionPane.showMessageDialog(null,"You entered an invalid keyword. Try again.", "Error!", 1); }

        }

        else
            JOptionPane.showMessageDialog(null,"You entered an invalid keyword. Try again.", "Error!", 1);


        if (pokemon.equalsIgnoreCase("Bulbasaur")) {
            region="Kanto";
            pokeNo="001";
            species="Seed";
            type="Grass";
            habitat="Grassland";
            desc="A strange seed was planted on its back at birth. The plant sprouts and grows with this Pokémon.";
            }

            else if (pokemon.equalsIgnoreCase("Charmander")) {
            region="Kanto";
            pokeNo="004";
            species="Lizard";
            type="Fire";
            habitat="Mountain";
            desc="Obviously prefers hot places. When it rains, steam is said to spout from the tip of its tail.";
            }

            else if (pokemon.equalsIgnoreCase("Squirtle")) {
            region="Kanto";
            pokeNo="007";
            species="Tiny turtle";
            type="Water";
            habitat="Water's-edge";
            desc="After birth, its back swells and hardens into a shell. Powerfully sprays foam from its mouth.";
            }

            else if (pokemon.equalsIgnoreCase("Chikorita")) {
            region="Johto";
            pokeNo="152";
            species="Leaf";
            type="Grass";
            habitat="Grassland";
            desc="A sweet aroma gently wafts from the leaf on its head. It is docile and loves to soak up the sun's rays.";
            }

            else if (pokemon.equalsIgnoreCase("Cyndaquil")) {
            region="Johto";
            pokeNo="155";
            species="Fire Mouse";
            type="Fire";
            habitat="Grassland";
            desc="It is timid, and always curls itself up in a ball. If attacked, it flares up its back for protection.";
            }

            else if (pokemon.equalsIgnoreCase("Totodile")) {
            region="Johto";
            pokeNo="158";
            species="Big Jaw";
            type="Water";
            habitat="Water's-edge";
            desc="Its well-developed jaws are powerful and capable of crushing anything. Even its trainer must be careful.";
            }

            else if (pokemon.equalsIgnoreCase("Treecko")) {
            region="Hoenn";
            pokeNo="252";
            species="Wood Gecko";
            type="Grass";
            habitat="Forest";
            desc="Its well-developed jaws are powerful and capable of crushing anything. Even its trainer must be careful.";
            }

            else if (pokemon.equalsIgnoreCase("Torchic")) {
            region="Hoenn";
            pokeNo="255";
            species="Chick";
            type="Fire";
            habitat="Grassland";
            desc="A fire burns inside, so it feels very warm to hug. It launches fireballs of 1,800 degrees F.";
            }

            else if (pokemon.equalsIgnoreCase("Mudkip")) {
            region="Hoenn";
            pokeNo="258";
            species="Mud Fish";
            type="Water";
            habitat="Water's-edge";
            desc="The fin on MUDKIP's head acts as highly sensitive radar. Using this fin to sense movements of water and air, this POKÉMON can determine what is taking place around it without using its eyes.";
            }

            else if (pokemon.equalsIgnoreCase("Turtwig")) {
            region="Sinnoh";
            pokeNo="387";
            species="Tiny Leaf";
            type="Grass";
            habitat="Lake-side";
            desc="It undertakes photosynthesis with its body, making oxygen. The leaf on its head wilts if it is thirsty.";
            }

            else if (pokemon.equalsIgnoreCase("Chimchar")) {
            region="Sinnoh";
            pokeNo="390";
            species="Chimp";
            type="Fire";
            habitat="Mountain";
            desc="Its fiery rear end is fueled by gas made in its belly. Even rain can't extinguish the fire.";
            }

            else if (pokemon.equalsIgnoreCase("Piplup")) {
            region="Sinnoh";
            pokeNo="393";
            species="Penguin";
            type="Water";
            habitat="Arctic";
            desc="A poor walker, it often falls down. However, its strong pride makes it puff up its chest without a care.";
            }

            else if (pokemon.equalsIgnoreCase("Snivy")) {
            region="Unova";
            pokeNo="495";
            species="Grass Snake";
            type="Grass";
            habitat="Forest";
            desc="It is very intelligent and calm. Being exposed to lots of sunlight makes its movements swifter.";
            }

            else if (pokemon.equalsIgnoreCase("Tepig")) {
            region="Unova";
            pokeNo="498";
            species="Fire Pig";
            type="Fire";
            habitat="Grassland";
            desc="It can deftly dodge its foe's attacks while shooting fireballs from its nose. It roasts berries before it eats them.";
            }

            else if (pokemon.equalsIgnoreCase("Oshawott")) {
            region="Unova";
            pokeNo="501";
            species="Sea Otter";
            type="Water";
            habitat="Unknown";
            desc="The scalchop on its stomach isn't just used for battle - it can be used to break open hard berries as well.";
            }

            else if (pokemon.equalsIgnoreCase("Chespin")) {
            region="Kalos";
            pokeNo="650";
            species="Spiny Nut";
            type="Grass";
            habitat="Unknown";
            desc="The quills on its head are usually soft. When it flexes them, the points become so hard and sharp that they can pierce rock.";
            }

            else if (pokemon.equalsIgnoreCase("Fennekin")) {
            region="Kalos";
            pokeNo="653";
            species="Fox";
            type="Fire";
            habitat="Unknown";
            desc="Eating a twig fills it with energy, and its roomy ears give vent to air hotter than 390 degrees Fahrenheit.";
            }

            else if (pokemon.equalsIgnoreCase("Froakie")) {
            region="Kalos";
            pokeNo="656";
            species="Foam Frog";
            type="Water";
            habitat="Unknown";
            desc="It secretes flexible bubbles from its chest and back. The bubbles reduce the damage it would otherwise take when attacked.";
            }

        else {
            JOptionPane.showMessageDialog(null, "No such Starter Pokemon exists. Try again."); }

        JOptionPane.showMessageDialog(null, "Pokemon: " + pokemon + "\nRegion: " + region + "\nNational Pokemon #: " + pokeNo + "\nSpecies: " + species + "\nType: " + type + "\nHabitat: " + habitat + "\nDescription: " + desc, "Pokedex Results", 1);


        }

    }

I just wanted to ask for suggestions on how to improve this code. I know it looks sort of messy and I really wanted to make use of arrays, except for a program like this, it seems that I would be needing tables, but we haven't discussed that in class yet and we were advised not to get ahead of ourselves.

I wanted to use loops for this too, to make it a little more complex, but I have no idea where I can fit all that in.

Also, this is a really huge bulk of codes (the longest code I've written so far), and I am aware that in Java, the simpler the code, the more efficient it is to use. Is there any way I can simplify this code without having to stray from the basics and still getting my desired output?

P.S. If you spot any mistakes, PLEASE feel free to correct them.

share|improve this question
2  
Have you learned about methods and classes? –  Simon André Forsberg Dec 12 '13 at 18:24
5  
Where's Pikachu? –  Mat's Mug Dec 12 '13 at 18:32
1  
@SimonAndréForsberg Yes. We've learned about creating methods and calling them in from a different class. –  braindead Dec 12 '13 at 18:35
    
@retailcoder Haha! I was basing the results on the games. They are usually only the three basic types: fire, water and grass. –  braindead Dec 12 '13 at 18:37
3  
"we were advised not to get ahead of ourselves" - please tell your teacher that the Code Review community respectfully disagrees with his methods. –  Simon André Forsberg Dec 12 '13 at 18:54
show 2 more comments

4 Answers

Your first problem that everything takes place in the main method. Much of the user interaction and initialization should be separated out into seperate methods to remove clutter.

Then, you are confusing data with code. You have about a dozen snippets like:

else if (pokemon.equalsIgnoreCase("Totodile")) {
    region="Johto";
    pokeNo="158";
    species="Big Jaw";
    type="Water";
    habitat="Water's-edge";
    desc="Its well-developed jaws are powerful and capable of crushing anything. Even its trainer must be careful.";
}

First, we should create a Pokemon object that represents each Pokemon:

class Pokemon {
    final String name;
    final String region;
    final int    pokeNo;
    final String species;
    final String type;
    final String habitat;
    final String description;

    Pokemon(String name,
            String region,
            int pokeNo,
            String species,
            String type,
            String habitat,
            String description) {
        this.name        = name;
        this.region      = region;
        this.pokeNo      = pokeNo;
        this.species     = species;
        this.type        = type;
        this.habitat     = habitat;
        this.description = description;
    }
}

Such code is rather ugly in Java, but it will make our code cleaner in a moment.

Next, we create a data structure that maps names to Pokemon objects. For now, we will require that proper capitalization of the name is used.

Map<String, Pokemon> pokemonsByName = new HashMap<String, Pokemon>();

then we can populate this data strucure, e.g.

pokemonsByName.put(
    "Totodile",
    new Pokemon(
        "Totodile",
        "Johto",
        158,
        "Big Jaw",
        "Water",
        "Water's-edge",
        "Its well-developed jaws are powerful and capable of crushing anything. "
        + "Even its trainer must be careful."
    )
);

When we have done all of this for all Pokemon, we can query for Pokemons by name:

Pokemon requestedPokemon = pokemonsByName.get(pokemonName);
if (requestedPokemon == null) {
  // no such Pokemon was found
} else {
  // we have found a pokemon, so display it.
}

But this makes our code longer! Why should we do this then? Well, as you add more and more Pokemon, you may not want to recompile your app each time. Instead, you might load all Pokemon stats from a CSV file at startup (there are open source libraries that will help you with this). Now you only have to edit the data file and restart the program in order to support new Pokemons.

Then you can add further improvements. For example, there are only a limited number of Pokemon types. Instead of using a String (which could contain typos), we can use an enum (enumeration) that only supports certain values:

enum PokemonType {
  WATER, FIRE, GRASS;
}

In the Pokemon class, you would then change the type of the type field from String to PokemonType. The constructor would then be invoked like

new Pokemon(
  ...,
  PokemonType.WATER, // instead of the string "Water"
  ...
);
share|improve this answer
    
I tried this method, actually! Except I don't really get Hashmaps yet and we haven't really discussed anything beyond Advance GUI in class. I particularly enjoy enum for being so handy but our instructor may not consider this code given that I strayed from what we were thought (the basics). This project is actually really just an application of what we've learned so far, but thank you for this comprehensive answer! Might make my own complete Pokedex in the near future using this one. Many thanks. –  braindead Dec 12 '13 at 18:42
    
Would it be possible to use enum for all the PokemonType? –  rolfl Dec 12 '13 at 20:20
    
@rolfl I don't quite understand that question. It is not possible to specify all properties of a Pokemon object via enums, as some are unique to the individual. –  amon Dec 12 '13 at 20:22
    
@amon - I meant Pokemon Type, not PokemonType ... i.e. public enum Pokemon { Totodile("Johto","....",....), Bulbasaur("...",...), ....} (with the correct Enum constructor, of course). –  rolfl Dec 12 '13 at 20:27
1  
@rolfl This would be possible, but I would advise against that. Not using an enum allows us to load the data from an external resource (e.g. a JSON document or CSV file), which is more flexible. –  amon Dec 12 '13 at 20:29
show 3 more comments

Let's break down your inputs:

  • Do you know the name of the Starter Pokemon you are looking for?
    • Yes: Enter the name of the Starter Pokemon
    • No: Search by: 'type' or 'region'?
      • Type: Enter pokemon type: (Fire/Water/Grass)
      • Region: Enter region: (Kanto/Johto/Hoenn/Sinnoh/Unova/Kalos)

Not very complex. What makes working with your code annoying, is all those if-else branches and the fact that you find an else branch several dozen lines under the if it belongs to.

Prompting for user input is one concern; put that in its own method - take a string for the title and another for the actual prompt, and an array of strings for validating the input; make that method return an empty string or a valid input.

In your search prompts, you're listing types and regions - don't hard-code them into the prompt, fetch them from your data instead (whether that's an enum or a csv or some xml you grabbed from a web service).

When you have a method that prompts for pokemons of a given type, take that type as a parameter, and fetch the pokemons that match that criteria instead of hard-coding them into your prompt string.

Same for regions: take that as a parameter, and fetch the pokemons that match that criteria.

This should leave you with much less redundant prompting code.

Once you have the pokemon you want, take @amon's advice and just fetch the appropriate Pokemon object - don't mix code with data.

Sorry this isn't a code-oriented answer, I don't do much :)

share|improve this answer
    
I wonder if there's a web service that spits out XML-serialized pokemons... –  Mat's Mug Dec 12 '13 at 20:36
    
well, I found JSON: pokeapi.co/docs –  IamAlexAlright Dec 13 '13 at 0:12
2  
and don't ask me why I even decided to look but you can query bulbapedia like this: bulbapedia.bulbagarden.net/w/… (see mediawiki.org/wiki/API:Query#Sample_query) –  IamAlexAlright Dec 13 '13 at 0:20
    
@IAmAlexAlright you're kidding me! Now I have to do something about that API!! –  Mat's Mug Dec 13 '13 at 0:23
3  
Hey guys, I'm the creator of PokéAPI, if you need any help or support let me know. Also - if you find bugs or want to suggest any features then let me know! –  phalt Dec 13 '13 at 10:41
show 3 more comments

Look out for

if (pokemon.equalsIgnoreCase("Bulbasaur")) {

This might produce a null pointer exception if it's not brought into the condition like this

if (pokemon != null && pokemon.equalsIgnoreCase("Bulbasaur")) {

The actual answer for this potential problem is actually switching the comparison to

if ("Bulbasaur".equalsIgnoreCase(pokemon)) {

This way you can be sure that a String "Bulbasaur" will never be null and you can compare other stuff to it.

share|improve this answer
1  
An assert is not triggered in production/release and as such are useless as a null-check. –  Max Dec 13 '13 at 9:36
    
Good point Max, I'll leave that out. –  Dropout Dec 13 '13 at 9:37
add comment

About nested if statements:

void f()
{
    if(cond1) {
        ...
    }
    else if(cond2) {
             ...
         }
         else {

         }
}

Can be rewritten as:

void f()
{
    if(cond1) {
        ...
        return;
    }
    if(cond2) {
        ...
        return;
    }
    ...
}

Which in my opinion is considerably more readable.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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