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.

This game is explained here.

Could this code be more efficient?

The Game Class

import java.util.List;
import java.util.Arrays;
import java.util.Collections;

public class Game {

    private Case[] cases = new Case[26];
    private Player player = new Player();
    private Banker banker = new Banker();
    private int myAmount = 0;
    private int turn = 1;
    private int briefCases = 26;
    private int casesToRemove = 6;
    private int offer = 0;
    private boolean gameContinue = true;

    public void caseSetup() {

        List<Integer> values = Arrays.asList(1, 5, 10, 100, 500, 750, 1000, 20000, 50000, 75000, 95000,
                100000, 150000, 250000, 350000, 400000, 450000, 500000, 600000, 700000, 25, 900000, 1000000, 1500000, 300, 900000);

        Collections.shuffle(values);

        for (int i = 0; i < cases.length; i++) {
            int amount = values.get(i);
            cases[i] = new Case(amount, i + 1);
        }
    }
    public void showCase(){
    for (int j = 0; j < cases.length; j++) {
            System.out.print("\t[" + cases[j].getFace() + "] ");
            if (j % 5 == 4) {
                System.out.println();
            }
        }
    }

    public void game(){
        caseSetup();
        showCase();

        myAmount = player.userCase(cases);
        briefCases--;

        while(gameContinue==true){

                if(briefCases == 25||briefCases == 19 ||briefCases == 14 || briefCases == 10 ||briefCases == 7){
                for(int counter = casesToRemove;counter>0;counter--){
                    player.removeCase(counter,cases);
                    briefCases--;
                    showCase();
                }
                offer = banker.getOffer(turn,cases,myAmount);
                gameContinue = player.dnd();
                casesToRemove--;
                turn++;
                }else if(briefCases == 1){
                    player.removeCase(1,cases); 
                    offer = banker.getOffer(turn,cases,myAmount);
                    gameContinue = player.dnd();
                    casesToRemove--;
                    briefCases--;
                    turn++;
                }
                else{
                    player.removeCase(1,cases); 
                    offer = banker.getOffer(turn,cases,myAmount);
                    gameContinue = player.dnd();
                    casesToRemove--;
                    briefCases--;
                    turn++;
                }
        }
        finishGame();
    }
        public void finishGame(){
        if(briefCases == 0){
        System.out.println("You've rejected the Offer of banker");
        System.out.println("You've won "+myAmount+"And your Case is Larger Than Bankers Offer: "+offer);
        }else{
        System.out.println("Your Accept the offer of banker");
        System.out.println("You've won "+myAmount+" and the Bankers offer is :"+offer);
        }
    }

}

Banker Class

public class Banker{

    private int total = 0;
    private int numOfSamples = 0;
    private int average = 0;
    private int offer = 0;

    public void setOffer(int turn,Case[] cases,int myAmount){
        for(int i = 0 ;  i < cases.length;i++){
            if(!cases[i].Removed()){
             total = myAmount+cases[i].getValue();
             numOfSamples++;
            }
        }
        average = myAmount+total/numOfSamples;
        offer = average*turn/10;
    }

    public int getOffer(int turn,Case[] cases,int myAmount){
     setOffer(turn,cases,myAmount);
     System.out.print("The Bankers Offer Is: "+offer);
     return offer;
    }
}

The Briefcase Class

public class Case{

    private int value = 0;
    private String face;
    private boolean removed = false;

    public Case(int nValue,int nFace){
        this.value = nValue;
        this.face = Integer.toString(nFace);
    }

    public int getValue(){
        return value;
    }

    public String getFace(){
        return face;
    }

    public boolean Removed()
    {
        return removed;
    }

    public void remove(){
        removed = true;
        face = "X";
    }
}

Main class

public class GameTest{

    public static void main(String[] args){

        Game DND = new Game();
        DND.game();
    }
}
share|improve this question
    
You should google "oo lecture" or "oo design" Here's an example of a link you get from it. –  Eva Jul 9 '12 at 0:39

2 Answers 2

A few comments:

  • Try to keep the scope of your variables as low as possible. Don't not be afraid of local variables.
  • Consistently follow naming conventions. Sometimes your method names are in UpperCamelCase. That should be reserved for class and interface names only. Also method names should be verbs or verb phrases. (EDIT: This used to say variable names should be verbs. That is completely wrong, and it was a sleep-deprived mistake on my part to include that. Variable names should be lowerCamelCase descriptive nouns.)
  • Complicated if statement can almost always be refactored away. Sometimes they need to move to their own class, but most of the time you can get rid of them altogether. See Strategy Pattern, Replace Type Code with Subclasses, Abstract Factory Pattern
  • Whitespace is your friend. It makes your code prettier and easier to read.
    average = myAmount+total/numOfSamples should be
    average = myAmount + total / numOfSamples
  • Magic number suck. Firmly eschew them.
  • Comments are your friend. A lot of the stuff you had was confusing the first couple go throughs because of a lack of comments. They can help you come up with better variable and function names, do better decomposition, and pay more attention to refactoring (all for the low, low price of $0.00!)

    For every line of code in your program:

    • Think of a comment that explains both what you're doing and why you're doing it. You should only add comments regarding why your doing something to your actual code, but thinking about what your doing helps with the refactoring.
    • If it's a comment on a field or constant and it's not already redundant, consider refactoring your variable name so that the comment becomes redundant. If there's still a why explanation needed, then add a comment, otherwise your code should mostly be self-documenting.
    • Comments before functions should be considered required before your function is complete. There is always a why to each function. However, thinking about the what can help you refactor the names of your functions. Also if you can't think of a good short name for your function it may be a sign that it is doing too much. Decompose.
    • Comments before classes are much like comments before functions; they are required, they can help you refactor the names of your classes, and they help get rid of God Classes.
    • Comments within functions that don't seem redundant are probably an indicator for more decomposition unless your doing a complicated algorithm. Decompose.
  • This code is begging to be extendable. Text-based is fine to start with, but sometimes there's that nagging feeling that you should change it to a GUI (or change the text interface or w/e). Design with that in mind. Your code is so tightly coupled a few small changes to the UI would make you have to go through the whole thing. You probably want more classes to separate out the different functionalities. Consider MVC.
share|improve this answer
1  
variable names should be verbs? Did you mean function names? –  Jason Larke Feb 20 '13 at 6:03
    
@JasonLarke You are totally right. I was very sleep-deprived when I wrote this answer. Messed that bit up. Edited now. –  Eva Feb 20 '13 at 6:20
    
No dramas, I think we've all been there :) –  Jason Larke Feb 20 '13 at 6:31

gameContinue==true can just be gameContinue

It looks like you could just get rid of:

else if(briefCases == 1){
     player.removeCase(1,cases); 
     offer = banker.getOffer(turn,cases,myAmount);
     gameContinue = player.dnd();
     casesToRemove--;
     briefCases--;
     turn++;       
}

These lines can be moved out of the if and else blocks:

offer = banker.getOffer(turn,cases,myAmount);
gameContinue = player.dnd();
casesToRemove--;
turn++;

I would either rename the setOffer method or get rid of it completely. The name is deceptive, and it does not seem to be used outside of the class. Further, it requires an additional state variable. (Is the ever increasing numOfSamples a bug?).

You may want to rename void game to void startGame. Also, those look like they should be the only public methods of that class.

Strictly speaking, it looks like Case should be the only class which has any state variables (you do not give the code for Player) -- if you have the time, you may want to refactor that as it will make life so much better.

share|improve this answer
    
what do you mean refactor ? (posted the player class) –  Andrew Asmer Sep 14 '11 at 5:02
    
Sorry, got the German link... here is the English version: en.wikipedia.org/wiki/Code_refactoring –  Landei Sep 14 '11 at 8:59

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.