15
\$\begingroup\$

I've just started programming and read a book called Head First Java.

I thought it would be fun to make a training project that I can put all the stuff I learn into. Is the code I've written here just really bad or good code written for a beginner?

I've got 4 classes:

Main class:

public class Main {
    Player p = new Player();
    p.Decision();
} 

Player Class:

public class Player {

    int decision = 0;
    int exit = 1;
    int wd;

    int yesorno;

    boolean firstTimePlaying = true;

    Game g = new Game();
    Scanner s = new Scanner(System.in);

    public void Decision() {
        if (firstTimePlaying == true) {
            g.firstTime();
            firstTimePlaying = false;
        }

        while (true) {
            System.out.println("Go out for a walk with your dog: 1 | Play with your dog: 2");
            decision = s.nextInt();
            if (decision == 1) {
                g.OutForAWalk();
            } else if (decision == 2) {
                g.play();
            } else if (decision == 425) {
                System.out.println("This is a developer option!");
                System.out.println("This will make you change your dogs name and size!");
                System.out.println("Do you wish to continue?");
                yesorno = s.nextInt();
                if (yesorno == 1)
                    g.firstTime();

            } else {
                System.out.println("Error");
            }

            System.out.println("Do you want to continue? Yes: 1 No: 2");
            exit = s.nextInt();
            if (exit > 1)
                break;
        }
    }

Game Class:

public class Game {
    int seeingAhydrant = 0;
    int wantingToPoop = 0;
    int seeingAnotherDog = 0;
    int fun = 0;
    String dogName;
    int size;

    Dog dog = new Dog();
    Scanner s = new Scanner(System.in);
    int chooise = 1;

    public void firstTime() {
        System.out.println("Welcome to this dog owner sim game!");
        System.out.println("Please write the name of your new dog: ");
        dogName = s.nextLine();
        System.out.println("Please write the size of your new dog: ");
        size = s.nextInt();
        dog.setName(dogName);
        dog.setSize(size);
        System.out.println("Now the game starts!");
    }

    public void OutForAWalk() {
        System.out.println("Player takes dog out for a walk");
        while (chooise < 2) {
            wantingToPoop += 2;
            seeingAhydrant = (int) (Math.random() * 11);
            seeingAnotherDog = (int) (Math.random() * 11);

            if (seeingAhydrant > 4) {
                dog.pee();
            } else if (wantingToPoop > 4) {
                dog.poop();
                wantingToPoop = 0;
            } else if (seeingAnotherDog > 7) {
                System.out.println("Suddenly another dog appear!");
                dog.bark();
            } else {
                System.out.println("Nothing special happend on thoose 100 meters");
            }
            System.out.println("Walk another 100 meters? 1 = yes/2 = no");
            chooise = s.nextInt();
        }
        chooise = 0;
    }

    // Bug inside play method, sometimes the dog dosen't show feelings (amuse, bored etc.)
    public void play() {
        while (chooise < 2) {
            System.out.println("You play with your dog.");
            fun = (int) (Math.random() * 11);
            if (fun < 1) {
                dog.verybored();
            } else if (fun > 0 && fun < 5) {
                dog.bored();
            } else if (fun > 4 && fun < 8) {
                dog.amused();
                dog.wiggleTail();
            } else if (fun > 7 && fun < 11) {
                dog.veryamused();
                dog.bark();
                dog.wiggleTail();
            }

            System.out.println("Do you want to play more with your dog? yes=1/no=2");
            chooise = s.nextInt();
        }
        chooise = 0;
    }
}

Dog Class:

public class Dog {

    private String name;
    private int size;

    public void bark() {
        if (size > 4)
            System.out.println(name + ": Woof Woof");
        else
            System.out.println(name + ": yip yip");
    }

    public void pee() {
        System.out.println(name + ": *Sees a hydrant* *peeing*");
    }

    public void poop() {
        System.out.println(name + ": *poops*");
    }

    public void wiggleTail() {
        System.out.println(name + ": *Wiggles tail*");
    }

    public void verybored() {
        System.out.println(name + ": *Looks really bored*");
    }

    public void bored() {
        System.out.println(name + ": *Looks bored*");
    }

    public void amused() {
        System.out.println(name + ": Looks amused");
    }

    public void veryamused() {
        System.out.println(name + ": Looks very amused");
    }

    //Getters and setters
    public int getSize() {
        return size;
    }

    public void setSize(int s) {
        if (s < 1) {
            System.out.println("Size can't be less then one!");
            System.out.println("Setting size to one!");
            size = 1;
        } else if (s > 10) {
            System.out.println("Size can't be more then ten!");
            System.out.println("Setting size to ten!");
            size = 10;
        } else {
            size = s;
        }
    }

    public String getName() {
        return name;
    }

    public void setName(String n) {
        name = n;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Stubbing in support for Easter Eggs already, I see! \$\endgroup\$ – 200_success Jul 13 '14 at 23:42
  • \$\begingroup\$ Haha yeah i do ;) \$\endgroup\$ – Svante Jul 14 '14 at 0:16
12
\$\begingroup\$

Besides the comments you've already received, I'm adding some more:


Many of your variables can be marked as private final. All of them can be marked private and those who get initialized directly and never change should also be marked final. These include:

Game g = new Game();
Scanner s = new Scanner(System.in);

Using == true is redundant here:

if (firstTimePlaying == true) {

It is enough with

if (firstTimePlaying) {

Some of your variable names are a bit too short and does not properly describe what they are used for:

int wd;
Game g
Scanner s

g can be called game and s either scanner or input. There's no need to use such short variable names, unless you are starting to run out of power in your keyboard (which I doubt).

In fact, your wd variable doesn't seem to be used at all. If it would have been marked as private your IDE would have told you about that.


It is a good practice to always use braces

if (yesorno == 1)
        g.firstTime();

is better written as:

if (yesorno == 1) {
    g.firstTime();
}

Many bugs have occurred in many programs because braces were missed.


You don't mention that you have to press 1 or 2 here. As a user, I would probably press y.

System.out.println("Do you wish to continue?");
yesorno = s.nextInt();

...Speaking of that, I would recommend that you change the functionality so that the user can input y or n instead. Entering 1 or 2 for yes/no is counter-intuitive. You want to make it simple for the user, even if it means it will be more complicated for you.


This error message describes absolutely nothing about what went wrong:

System.out.println("Error");

If I would encounter such a non-describing error I would have thought that something went totally wrong in your program.

State instead "You entered an incorrect value, please input either a, b or c...".


425 is a magic and seemingly random number. Magic numbers should not be entered in your code directly but instead used as named constants.

Instead, use this at the top of your class:

private static final int DEBUG_OPTION = 425;

And then in your code, use:

} else if (decision == DEBUG_OPTION) {

You have several methods that calls System.out.println(name + ": ...");

public void pee() {
    System.out.println(name + ": *Sees a hydrant* *peeing*");
}

public void poop() {
    System.out.println(name + ": *poops*");
}

I consider this as code duplication, what if you wanted to change name + ": " to name + " says: "? At the moment you would have to modify several lines for that. Your code will be a lot more maintainable, and slightly more readable if you extract a method:

private void perform(String action) {
    System.out.println(name + ": " + action);
}

public void pee() {
    perform("*Sees a hydrant* *peeing*");
}

public void poop() {
    perform("*poops*");
}

public void wiggleTail() {
    perform("*Wiggles tail*");
}
\$\endgroup\$
  • \$\begingroup\$ Thanks you so much for your answer Simon! May i ask if you are from sweden? Your name sounds pretty swedish ^^ \$\endgroup\$ – Svante Jul 13 '14 at 21:36
  • \$\begingroup\$ @Svante Indeed I am Swedish. Come and say Hej to me and the other (non-Swedish) site regulars in The 2nd Monitor chatroom :) \$\endgroup\$ – Simon Forsberg Jul 13 '14 at 21:52
11
\$\begingroup\$

There are a number of things to be improved, but here's a start:

  1. Functions should be named according to what they do. This means they should often start with a verb, so rather than Game.OutForAWalk(), it should be Game.goOutForAWalk(). (There are exceptions. For example, when checking a condition, you can use .is*.) Furthermore, they should be named in camelCase; the first letter should be lowercase, while the first letter in each word should be uppercase.

  2. Consider aggregating your dog mood functions into a single Dog.printMood() method. You'll probably soon learn about enums; an enum would be a natural way to store and represent the dog's current mood (à la Dog.setMood(Dog.MOOD.HAPPY)).

  3. If you're looking for a 40% chance of something, then Math.random() < 0.4 or (int)(Math.random() * 10) < 4 will do what you want. fun = (int)(Math.random() * 11) will get you a random number from 0 to 10 inclusive, so fun < 4 will actually be a 36% chance.

  4. Your if chain is needlessly complex. Remember that an else if will only be considered if all preceding ifs and else ifs failed. Thus, you can instead use an if-else ladder similar to the following. (Note that the percentages are accurate only if you change your random line to use 10 rather than 11.

    if (fun < 1)      // 10% chance: 0
    else if (fun < 5) // 40% chance: 1, 2, 3, 4
    else if (fun < 8) // 30% chance: 5, 6, 7
    else              // 20% chance: 8, 9
    
  5. A condition like seeingAHydrant > 4 makes no sense. Consider changing changing that to:

    boolean seesAHydrant = Math.random() < 0.6; // 60% chance
    if (seesAHydrant) {
        ...
    }
    
  6. chooise should be either choice or choose. In this case, choice makes more sense.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer! I will look into the things you wrote and try to change my code! :) \$\endgroup\$ – Svante Jul 12 '14 at 20:04
  • \$\begingroup\$ @Svante: Just don't change the code you've posted, please. It must stay intact after answers have been posted. \$\endgroup\$ – Jamal Jul 12 '14 at 20:07
  • \$\begingroup\$ @Jamal No, i meant the code i have in my IDE. \$\endgroup\$ – Svante Jul 12 '14 at 20:18
  • \$\begingroup\$ @Svante: Okay, just making sure. \$\endgroup\$ – Jamal Jul 12 '14 at 20:18
  • 1
    \$\begingroup\$ "(Note that the percentages are accurate only if you change your " your what? \$\endgroup\$ – Simon Forsberg Jul 13 '14 at 13:19

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.