Bear and Candies Game

By Posted on

Problem

I am trying to improving my Java skills and I’ve been doing various different projects/challenges which I usually am able to complete. I unfortunately never have my code reviewed so while my code may work, I would love to hear ways to do things better. I went on Codechef and solved one of the problems and added some basic error handling in addition to what the problem asked. I tried to write the code as clearly as possible to avoid unnecessary comments.

Main.java

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class Main {

static List<Integer> maxEatingCaps_Limark = new ArrayList<>();
static List<Integer> maxEatingCaps_Bob = new ArrayList<>();

public static void main(String[] args) {

    setUp();
    Bear Limak = new Bear(maxEatingCaps_Limark,"Limak");
    Bear Bob = new Bear(maxEatingCaps_Bob,"Bob");
    Bear.startEating(Limak,Bob);

    }


public static void setUp() {
    int  numOfTestCases = 0;
    Scanner input = new Scanner(System.in);
    System.out.println("Enter number of test cases");

    try {
        numOfTestCases = Integer.parseInt(input.nextLine().trim());
    }
    catch (NumberFormatException e) {
        System.err.println("You must enter an number" );
        System.exit(1);
    }

    boolean validInput = false;
    for (int i = 0; i < numOfTestCases ; i++) {
        validInput = false;

        while (!validInput)
            try {

                String maxEatingInput = input.nextLine();
                String[] individualEatingCaps = maxEatingInput.trim().split("\s+");
                maxEatingCaps_Limark.add(Integer.parseInt(individualEatingCaps[0]));
                maxEatingCaps_Bob.add(Integer.parseInt(individualEatingCaps[1]));
                validInput = true;

            } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
                System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
                validInput = false;

            }

    }

}

Bear.java

import java.util.List;


public class Bear {

    private static int amountOfCandyToEat;
    private static boolean haveWinner;
    private static String currentLeader;
    private int candyConsumed;
    private List<Integer> maxEatingCapacity;
    private String name;
    private static int numOfTest;



    Bear(List<Integer> maxEatingCapacity, String name) {

        this.maxEatingCapacity = maxEatingCapacity;
        this.name = name;
        candyConsumed = 0;
        numOfTest = maxEatingCapacity.size();
    }

  public static void  startEating(Bear bear1, Bear bear2) {

        for (int i = 0; i < numOfTest ; i++) {

            amountOfCandyToEat = 1;
            haveWinner = false;
            currentLeader = null;
            bear1.reset();
            bear2.reset();

            while(!haveWinner) {

                tryEating(bear1,i);
                tryEating(bear2,i);
            }
            System.out.println(currentLeader);

        }
        }
    private void reset() {
        candyConsumed = 0;
    }

    private static void tryEating(Bear bear, int index) {

        if (bear.getCandyConsumed() + Bear.getAmountOfCandyToEat() <= bear.getMaxEatingCapacity(index)) {
            bear.setCandyConsumed(bear.getCandyConsumed() + Bear.getAmountOfCandyToEat());
            Bear.setCurrentLeader(bear.getName());
            amountOfCandyToEat++;
        }
        else{
            haveWinner = true;
        }
    }


  private String getName() {
      return name;
  }

  private static int getAmountOfCandyToEat() {
      return amountOfCandyToEat;
  }

  private int getMaxEatingCapacity(int index) {
      return maxEatingCapacity.get(index);
  }

  private int getCandyConsumed() {
      return candyConsumed;
  }

  private void setCandyConsumed(int candyConsumed) {
      this.candyConsumed = candyConsumed;
  }

  private static void setCurrentLeader(String currentLeader) {
      Bear.currentLeader = currentLeader;
  }
}

Solution

Where to start …
I’d say, in general, I can follow the code and understand it. The naming of your variables are quite decent, that really helped a lot.

But there’s a lot of painful stuff around. I focus on those which hurt the most – many things should also be obsolete after few changes I recommend, too.

static variables vs instance variables

You mix a lot static and instance variables – I was actually really surprised, the code worked! I don’t think you have understood that concept, because in the Bear class, for example in the tryEating method, you mix everything up. You also access static variables directly and at the same time, you use the static getter method.

If you declare a variable static, it is “bound” to the class, but not to the instance – the instance can access those of the class, but not the other way around. But: Those statics in Bear shouldn’t be there in the first place (explained later). In general, static variables are rarely used, if a “variable” is declared static, it’s usually a constant. Static methods are used more often, but those are usually ‘utility’ methods of a class, which work independently of the state of a class.

separation of concerns

One of the main object oriented principles: A class should do “only one thing”. The more it does, the more complicated it gets, the harder it is to find bugs or change it. The Bear does on one hand hold its state (the name, the candy consumed), but on the other hand, does also the game logic (I’m just gonna call it like that now)! The game logic must be moved to a separate type. And then, you can also get rid of all this static variables. At the same time, the Main class loads and starts the game, but also takes user-input. That should be refactored to a separate type, too. Maybe do a class with a method like loadGameConfiguration, which returns a GameConfiguration.

I think, if you get rid of the missuse of static/instance variables and separate the concerns, you will get the most improvement.

Formatting / Naming

There’s some bad formatting – indentions, empty lines. Get used to format your code before you save (or use a ‘save action’ in your IDE), it helps a lot. Another problem is fixing the formatting and commit it to the code repository – understanding changes between commits gets more difficult.

Also check out the java naming conventions, how variables should / must be named.

Code Style

Now the small, but important details.

Main constructor

setUp: Setup what? A method should be as self explanatory as possible. It shouldn’t be too long – if that’s the case, it’s usually a hint, that a method does too many things (“do only one thing”) and should be split up.

Main.setUp()

  • numOfTestCases: Put your variables as close to the point to where its used, it avoids jumping around in the code. When you read the code, and the declaration of a variable is “too far away”, you can’t read it fluently.
  • boolean validInput: You actually declare the value and assign the same value inside of the for loop. There’s no need for that. It’s only used within the loop, so no need to put it outside of it, anyway.
  • It actually does two things, it loads the amount of test cases, but also the eating caps of the bears, you can do that in two separate methods (as mentioned, all that should be moved to a separate type anyway).

Bear constructor

  • numOfTest: You assign the size of the list of the eatingCap to numOfTest. Now, the bad thing about that is, it’s an assumption. You assume, that the amount of test is the size of the passed list. I can’t exactly explain why this is a bad thing, but “carrying assumptions through the code” will backfire: If the implementation changes, so the size of the list is not what you assume, but the amount of tests must be the amount the user did enter, you have a bug.
  • candyConsumed = 0: It’s perfectly fine to assign the value at the declaration on top – it makes no difference to the code, but the constructor is usually more clear that way.
  • maxEatingCapacity: At other places, you call it maxEatingCap: Try to be consistent.

Bear.startEating

Okay, that’s the most confusing part. The control flow does not make much sense, neither does the access to static variables. In general, everything here is quite confusing, to say the least.

  • The control flow is way too complicated. Why not something like “when a bear can eat, then eat, otherwise, the bear hast lost”? You wrote “as long as we have no winner – try to eat”. I have so many questions! Try to write a summary of the story you want to tell. Move the details of the story to private methods. You actually moved parts of the main flow – the summary – away. that makes it much harder to understand.

  • Bad naming. It’s actually a complete run of the game.

  • amountOfCandyToEat: Better would be currentAmountOfCandyToEat.

Bear.tryEating

  • The if statement reads as follows: “When the consumed candy of the bear plus the current amount of candy to eat is smaller that the bear’s max eating capacity”. If you’d move that condition to a separate method like canBearEat, it would be so much easier to read. Again: I want first to have a summary, if I want to know the details, I can jump into the private method.

Hope that helps…

Helper methods

    int  numOfTestCases = 0;
    Scanner input = new Scanner(System.in);
    System.out.println("Enter number of test cases");

    try {
        numOfTestCases = Integer.parseInt(input.nextLine().trim());
    }
    catch (NumberFormatException e) {
        System.err.println("You must enter an number" );
        System.exit(1);
    }

You can rewrite this as 

    Scanner input = new Scanner(System.in);
    System.out.println("Enter number of test cases");

    int remaining = inputInteger(input);
    input.nextLine();

With 

public static void inputInteger(Scanner input) {
    try {
        return input.nextInt();
    } catch (NumberFormatException e) {
        System.err.println("You must enter a number" );
        System.exit(1);
    }
}

This saves having to separate declaration and initialization of the variable. Now you can do both in one line.

I prefer the name remaining to numOfTestCases. Why may become clearer when you see how I use it.

You don’t need to jump through hoops to get an integer with a Scanner. There’s a method just for that.

Don’t overcomplicate things

    boolean validInput = false;
    for (int i = 0; i < numOfTestCases ; i++) {
        validInput = false;

        while (!validInput)
            try {

                String maxEatingInput = input.nextLine();
                String[] individualEatingCaps = maxEatingInput.trim().split("\s+");
                maxEatingCaps_Limark.add(Integer.parseInt(individualEatingCaps[0]));
                maxEatingCaps_Bob.add(Integer.parseInt(individualEatingCaps[1]));
                validInput = true;

            } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
                System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
                validInput = false;

            }

    }

}

You don’t need validInput. Consider 

    while (remaining > 0) {
        try {
            String maxEatingInput = input.nextLine();
            String[] individualEatingCaps = maxEatingInput.trim().split("\s+");
            maxEatingCaps_Limark.add(Integer.parseInt(individualEatingCaps[0]));
            maxEatingCaps_Bob.add(Integer.parseInt(individualEatingCaps[1]));
            remaining--;
        } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
            System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
        }
    }

This saves you two variables and a loop and gives the same result.

Note that you could also drop down to 

    while (remaining > 0) {
        try {
            maxEatingCaps_Limark.add(input.nextInt());
            maxEatingCaps_Bob.add(input.nextInt());
            remaining--;
        } catch (NumberFormatException e) {
            System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
        }
    }

But this doesn’t enforce the two numbers per line requirement. Up to you if you care.

Equations over iteration

You don’t have to iterate through the candy. You can do the quadratic formula instead.

(n+1)24=m

(n+1)24=m

Where n

n

is the number of candies that first bear eats and m

m

is the maximum for that bear. Solving for n

n

gives

n=2m−−√−1

n=2m−1

Rounding down to an odd integer gives the last number of candies that bear can eat. Add 2 to give the first number that it can’t eat. Now do the equivalent for the second bear.

n(n+1)=m

n(n+1)=m

And the quadratic formula gives

n=−1±1+4m−−−−−−√2

n=−1±1+4m2

Negative numbers aren’t meaningful here, so

n=4m+1−−−−−−√−12

n=4m+1−12

Round down to an even integer to get the last number of candies that can be eaten.

Whoever gets farther wins.

Now you just have to turn those equations into Java.

Leave a Reply

Your email address will not be published. Required fields are marked *