1
\$\begingroup\$

I am looking for help with my code, new to programming/development and looking for advice on how to improve.

I’ve been having issues with objects made in do loops, unable to access them outside of the loop. I’m not sure how to continue and I am looking for advice and suggestions or example programs for reference.

import java.util.Scanner;

public class Dungeon {  
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);    
        System.out.println("Welcome to The Dungeon" +
                        "\nTo create a new character enter 1" +
                        "\nTo load a old character enter 2");
        boolean check = false;
        int i = 0;      
        do {
            try {
                i = Integer.parseInt(sc.nextLine());
            }
            catch (Exception e) {
                check = true;
                System.out.println("Please input a valid input");
            }
            switch (i) {
                case 1: {
                    Character player = new Character();
                    player.factory();
                    check = false;
                    break;
                }
                case 2: {
                    System.out.println("Working");
                    Character player = new Character();
                    player.factory();
                    check = false;
                    break;
                }
                default: {
                    System.out.println("Please input a '1' or '2'");
                    check = true;
                    break;
                }
            }
        } while(check);
        System.out.println("end");



    }
}



import java.util.Scanner;

public class Character {
    private String name;
    private String profession;
    private int endurance;
    private int intellect;
    private int strength;
    private int agility;


    public Character () {

    }

    public void factory() {
        boolean check;
        int i = 0;
        do {
            createName();
            createProfession();
            displayStats();
            System.out.println("Enter '1' to confirm character"
                            + "\nEnter '2' to restart character creation");
            i = intGet();
            switch (i) {
                case 1: {
                    check = false;
                    break;
                }
                case 2: {
                    check = true;
                    break;
                }
                default: {
                    System.out.println("Please input a '1' or '2'");
                    check = true;
                    break;
                }
            }
        } while (check);
    }


    public void createName() {
        String pName = "";
        boolean check;
        do {    
            System.out.println("What is your name?");
            pName = stringGet();
            System.out.println("Is this your name? \n" +
                                "     '" + pName + "'" +
                                "\nIf 'Yes' press '1' if 'No' press '2'");
            int i = 0;
            i = intGet();

            switch(i) {
                case 1: {
                    check = false;
                    break;
                }
                case 2: {
                    check = true;
                    break;
                }
                default: {
                    System.out.println("Please input a '1' or '2'");
                    check = true;
                    break;
                }
            }
        } while (check);
        name = pName;
    }

    public void createProfession() {
        System.out.println("What is your profession?"
                        + "\nPress 1 for Fighter"
                        + "\nPress 2 for Mage"
                        + "\nPress 3 for Archer");

        int i = 0;
        boolean check;
        do {
            i = intGet();
            switch(i) {
                case 1: {
                    profession = "Fighter";
                    endurance = 10;
                    intellect = 2;
                    strength = 10;
                    agility = 0;
                    check = false;
                    break;
                }
                case 2: {
                    profession = "Mage";
                    endurance = 3;
                    intellect = 10;
                    strength = 2;
                    agility = 7;
                    check = false;
                    break;
                }
                case 3: {
                    profession = "Archer";
                    endurance = 5;
                    intellect = 2;
                    strength = 5;
                    agility = 10;
                    check = false;
                    break;
                }
                default: {
                    System.out.println("Please input a '1', '2' or '3'");
                    check = true;
                    break;
                }
            }
        } while (check);
    }

    public int intGet() {
        Scanner sc = new Scanner(System.in);
        int i = 0;
        try {
            i = Integer.parseInt(sc.nextLine());
        }
        catch(Exception e) {
            System.out.println("Please input a valid input");
        }
        return i;
    }

    public String stringGet() {
        Scanner sc = new Scanner(System.in);
        String i = "";
        try {
            i = sc.nextLine();
        }
        catch(Exception e) {
            System.out.println("Please input a valid input");
        }
        return i;
    }

    public void displayStats()
    {
        System.out.println(name + " the " + profession + "'s Stats" );
        System.out.println("-----------------------------");
        System.out.println("Endurance:\t\t" + endurance);
        System.out.println("Intellect:\t\t" + intellect);
        System.out.println("Strength:\t\t" + strength);
        System.out.println("Agility:\t\t" + agility);
    }
}
\$\endgroup\$
  • \$\begingroup\$ "unable to access them outside of the loop" You may want to brush up on the scope of a variable. The magic keywords are "variable scope". \$\endgroup\$ – Mast Dec 17 '17 at 20:29
1
\$\begingroup\$

First, I'll oversimplify an answer to your question: you can approximate a variable's scope by searching backwards from the declaration for the first unmatched open curly brace:

    do {               // <-- This is the first unmatched open curly.
        if (stuff) {   // <-- This is a matched open curly.
             statements;
        }

        int i = 0;     // <-- This is a variable declaration.

The first unmatched curly is usually the beginning of scope in curly-brace languages (C, C++, D, Java, Perl, etc.). The corresponding closing curly is usually the end of scope.

There are special cases in some of these languages. But they're special. You will go far if you just assume that your scope ends when the enclosing curly braces are closed. This means that variables used in the control expressions of loops should be declared (& initialized!) outside the loop body. And it means that any "result" you want to carry out of the loop must also be declared outside the loop body.

Structure

With that said, let's talk about the structure of your code. I see you have an intGet function defined in class Character. Have you noticed that it's the wrong place?

You have several classes that want to "talk" to the user. That's a problem- it's hard to write a intGet function in several different classes.

Instead, let me suggest you create a TextUI class that can display messages and present the user with a menu of options and return a valid item from that menu. It may also query the user for non-empty strings, and whatever other things you need it to do.

You can pass an instance of this class to your other classes so that they can use it to communicate with the user. But I'd recommend you create separate classes or methods for things like constructing a Character. Define a "createCharacter(textui)" method on Dungeon that talks to the user and then calls Character.factory() with the character-class, name, and whatever user-given inputs you want.

Subclass, don't switch

When you find yourself writing a switch statement in an OO program, ask yourself, "Does this indicate a subclass?"

In your case:

        switch(i) {
            case 1: {
                profession = "Fighter";
                endurance = 10;
                intellect = 2;
                strength = 10;
                agility = 0;
                check = false;
                break;
            }

This indicates a subclass. Instead of a constructor for Character, you should have constructors for Fighter and Mage and Archer and whatever other classes there are. Your Character class can provide the list of common methods and instance data, as well as a factory method that takes a class name, switches on it and returns the appropriately typed new object:

public static Character factory(String playerclass, String name) {

    if (playerclass.equals("Fighter")) return new Fighter(name);
    if (playerclass.equals("Mage"))    return new Mage(name);
    if (playerclass.equals("Archer"))  return new Archer(name);
    throw ...
}

Separate your concerns

One of the core values of OO programming is "separation of concerns." If your code is trying to manage user input and trying to do something else, then your concerns are not separated. All those try/catch statements for parsing integers, and loops for validating input are potential bugs. Push all that into the TextUI class, and write the code one time. If your TextUI doesn't have a method for doing "X", then write a .doX method! Maybe you'll see that two slightly different things could be merged together.

The idea here is that you should be able to simply call a method and trust the result! If you don't feel like you can trust the result, you get the kind of code you have now - a mishmash of concerns. Wrap that untrustable stuff in a method that does what you need, and then trust that method.

The same is true for your other subclasses. When Dungeon needs a character created, it should be able to make a call and trust the result. If there needs to be drama, or dialog, or uncertainty, try to get that taken care of before you call the factory method. (Hence, a method to talk to the user and then create the character.)

If your code is only doing one thing at a time, you'll find it much easier to write.

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your help, I had some further questions. When implementing the stats for characters and monsters should I use an array to hold the stats or keep them as variables? What would the differences be? (I know this is really broad, not sure how to better phrase it) When I'm creating new character classes and monsters should I just make them whole new classes with their own constructors and just extend them out of the 'Monster' class or should I just change their name and use a method to change the stats and name within the class? How do you implement the 'throw' section \$\endgroup\$ – David Jorgensen Dec 27 '17 at 2:33
  • \$\begingroup\$ The key question is, do you need the stats to be an array? I think the answer is 'no' but it's up to you. For example, if you frequently find yourself processing the stats in order, then a loop would be good and so an array or list makes sense. But for most games of this type, that's not the case- you typically compute using player.str or monster.dex and call out the stats by name. For the subclass question, I'd say try writing two or three subclasses of monster. Then see if they are "really different" and so subclasses, or if you can fold them back in using some kind of init-parameter. \$\endgroup\$ – Austin Hastings Dec 27 '17 at 3:01
  • \$\begingroup\$ As for how to implement the throw section, that's up to you. Really, there shouldn't be one - because you should never call your factory with an invalid subclass. But that's an exception - it detects when you did something crazy, like call your factory with an invalid subclass! So I'd suggest you throw a IdiotProgrammerException with an explanation of the problem. \$\endgroup\$ – Austin Hastings Dec 27 '17 at 3:05

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.