HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Dog owner simulator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
ownersimulatordog

Problem

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!");

Solution

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*");
}

Code Snippets

Game g = new Game();
Scanner s = new Scanner(System.in);
if (firstTimePlaying == true) {
if (firstTimePlaying) {
int wd;
Game g
Scanner s
if (yesorno == 1)
        g.firstTime();

Context

StackExchange Code Review Q#56866, answer score: 12

Revisions (0)

No revisions yet.