Critique my learning project

[coolbeanbob]

I would like thoughts/advice on the project I am working on. I am trying to build a program that simulates the score of a baseball game given individual player statistics. As of right now, the program only has one team batting for nine innings.

I am a computer science student and I am using this as a learning project. I find this project interesting, and I would like to eventually use this as evidence of my programming skills to potential employers.

So here are a few questions I have for someone with more experience than me...

• Am I using too many classes?
• Do you like the way I have structured the classes or is there a better alternative?
• In terms of difficulty, how does this compare to the projects a typical software engineer would work on from day to day? I assume this is not very difficult. In which ways could I expand the program?

I plan to link this program to a database and pull the statistics in at runtime. I also plan to add a GUI. Any other comments are appreciated!

public class Game {

    final static int VISIT = 0;
    final static int HOME = 1;

    public static void main(String[] args) {
        Field field;
        field = new Field();
        Roster homeRoster;
        Roster visitRoster;
        int pitchResult;
        visitRoster = field.getRoster(VISIT);
        homeRoster = field.getRoster(HOME);         
        Player vPitcher = visitRoster.getPlayer(8);
        Player hPitcher = homeRoster.getPlayer(8);      

        //Test test = new Test();


        do{

            Scorebook.setOuts(0);
            while(Scorebook.getOuts() < 3){
                Player batter = visitRoster.getBatter();        
                pitchResult = field.pitch(hPitcher, batter);
                if(pitchResult == 1){
                    field.advanceBases();
                }
                else Scorebook.increaseOuts();
                System.out.println("Inning: " + Scorebook.getInning() + " Score: " + Scorebook.getVisitScore() + " Outs: " + Scorebook.getOuts());  
            }
            Scorebook.increaseInning();


        }while((Scorebook.getInning() < 10) || (Scorebook.getHomeScore() == Scorebook.getVisitScore()));

        System.out.println(Scorebook.getVisitScore());  
    }



    // keep track of atBat
    // 




}




import java.util.Random;


public class Field {

    Base bases[];
    Base firstBase;
    Base secondBase;
    Base thirdBase;
    Base homeBase;

    Roster visitingTeam;
    Roster homeTeam;



    Field(){
        firstBase = new Base("First Base");
        secondBase = new Base("Second Base");
        thirdBase = new Base("Third Base");
        homeBase = new Base("Home Base");

        bases = new Base[4];
        bases[0] = firstBase;
        bases[1] = secondBase;
        bases[2] = thirdBase;
        bases[3] = homeBase;

        visitingTeam = new Roster();
        homeTeam = new Roster();

    }

    // 0 returns visitingRoster, 1 returns homeRoster
    public Roster getRoster(int i){
        if(i == 0){
            return visitingTeam;
        }
        else {
            return homeTeam;
        }
    }

    public int pitch(Player pitcher, Player batter){
        int hit = 0;
        Random randomGenerator = new Random();
        double randomDouble = (double)randomGenerator.nextInt(100)/100;
        if(randomDouble < batter.getBattingAvg()){
            hit = 1;
        }

        return hit;
    }

    public void advanceBases(){
        for(int i = 0 ; i < 3; i++)
        {   // player on third scores
            if(bases[i].getBaseStatus() == true){
                if(bases[i].getBaseStatus() == true){
                    bases[i].setBaseStatus(false);
                    Scorebook.increaseVisitorsScore();
                }
            }   
            else{
                if(bases[i].getBaseStatus() == true){
                    bases[i+1].setBaseStatus(true);
                    bases[i].setBaseStatus(false);  
                }   
            }       
        }
        bases[0].setBaseStatus(true);
    }






}





public class Scorebook {

    static private int inning;
    static private int outs;
    static private int homeScore;
    static private int visitingScore;



    Scorebook(){
        inning = 1;
        outs = 0;
        homeScore = 0;
        visitingScore = 0;
    }

    static public void increaseInning(){
        inning++;
    }

    static public void increaseOuts(){
        outs++;
    }

    static public void increaseHomeScore(){
        homeScore++;
    }

    static public void increaseVisitorsScore(){
        visitingScore++;
    }

    static public int getInning(){
        return inning;
    }

    static public int getOuts(){
        return outs;
    }

    static public int getVisitScore(){
        return visitingScore;
    }   
    static public int getHomeScore(){
        return homeScore;
    }

    static public void setOuts(int i){
        outs = i;
    }

/*  
    static public int getScore(int i){
        if (i == 0){
            return visitingScore;
        }
        else return homeScore;
    }
*/



}




import java.util.LinkedList;
import java.util.Queue;

public class Roster {
    final int ROSTERSIZE = 9;
    Player p0;
    Player p1;
    Player p2;
    Player p3;
    Player p4;
    Player p5;
    Player p6;
    Player p7;
    Player p8;
    Player roster[];

    Queue<Player> qe=new LinkedList<Player>();





    Roster(){ // Player name, battingAvg, era
        p0 = new Player("p1", .085, .350);
        p1 = new Player("p1", .075, .350);
        p2 = new Player("p2", .330, .350);
        p3 = new Player("p3", .275, .350);
        p4 = new Player("p4", .280, .350);
        p5 = new Player("p5", .260, .350);
        p6 = new Player("p6", .060, .350);
        p7 = new Player("p7", .050, .350);
        p8 = new Player("p8", .000, .350);

        roster = new Player[ROSTERSIZE];        
        roster[0] = p0;
        roster[1] = p1;
        roster[2] = p2;
        roster[3] = p3;
        roster[4] = p4;
        roster[5] = p5;
        roster[6] = p6;
        roster[7] = p7;
        roster[8] = p8;

        // queue for batting line up
        for(int i = 0; i < roster.length; i++){
            qe.add(roster[i]);
        }


    }

    public Player getPlayer(int i){
        Player temp;
        temp = roster[i];
        return temp;
    }

    public Player getBatter(){
        Player temp;
        temp = qe.remove();
        qe.add(temp);
        return temp;
    }



}





public class Player {

    String name;
    double battingAvg;
    double era;

    // primary constructor for creating a player
    Player(String playerName, double battingAverage, double pitcherEra){
        name = playerName;
        battingAvg = battingAverage;
        era = pitcherEra;
    }
    // constructor overloaded to take no arguments
    Player(){
        name = null;
        battingAvg = 0;
        era = 0;
    }

    public String getName(){
        return name;
    }

    public double getBattingAvg(){
        return battingAvg;
    }

    public double getERA(){
        return era;
    }










}





public class Base {

    String name;
    boolean base;

    Base(String n){
        name = n;
        base = false;
    }

    void fillBase(){
        base = true;
    }

    void clearBase(){
        base = false;
    }

    boolean getBaseStatus(){
        return base;
    }

    void setBaseStatus(boolean bool){
        base = bool;
    }


}

[Taywin]

Am I using too many classes?

No, you don't. A good way to do OOP is to break down a big object into smaller objects. Your classes look fine to me. If you feel that there are too few classes, you could still break it down further in the future. I don't see that there are too many for the current program.

Do you like the way I have structured the classes or is there a better alternative?

As I said earlier, they are fine. There are some issues (at least to me) that I will suggest later.

In terms of difficulty, how does this compare to the projects a typical software engineer would work on from day to day? I assume this is not very difficult. In which ways could I expand the program?

I think this is a good start for you. In term of difficulty for this project compare to an average small project I usually work on, from 1 to 10 where 1 is the easiest and 10 is the most difficult, I would rate this project as 3. The reason behind this score is that the program is straight forward. This does not mean it is bad, but it means it has a lot of rooms for improvement. At the current stage, it is very simple and does not require any algorithm to optimize or tweek it. In the future, you may need to apply that to. Also, the program has no GUI which is actually another skill you would want to acquire. No need to be hurry and shouldn't skip the fundamental steps. You need to be firm for your basic first, and you will see the results in the future.

One suggestion I want to give you is you should comment your code. At least, you need to comment what a class is supposed to be doing and used. Each method besides constructors, you should at least comment and give detail of what the method is for. You should also include its argument descriptions as well. This is important for others and you in the future. Keep doing it until it is your habit. Those who say comment is for noobs are thsoe who has no ability to work in a team of all levels.

Next suggestion is your Roster class. It contains so many hard-coded parts. If you want data that is fixed, I would rather read the data off from somewhere such as a file. Otherwise, I would use Random to randomize the stat. Also, you should use a loop to go through the creation rather than create multiple local variables.

    // i.e.
    // this is just a sample
    roster = new Player[ROSTERSIZE];
    Random rand = new Random();
    for (int i=0; i<roster.length; i++) {
      roster[i] = new Player("p"+(i+1), rand.nextFloat(), rand.nextFloat());
    }

Also, in the same class, I think getRoster() should be a static method instead of requiring to initantiate just to create a roster.

The last suggestion for now is in your Game class. Look at your constants -- VISIT and HOME. They are mutual exclusive -- either/or and never be both at the same time. It is a good idea to use a boolean which should not be a constant for these constants instead. Unless, the game allows more than 2 batting teams playing against one another. If so, your constant declration is fine, and could be taken out to another class in the future.

[coolbeanbob]

I really appreciate your advice. I have made the changes you suggested.

I have also linked the program up to an Access database. I was able to download a free database with baseball statistics. The website (http://www.seanlahman.com/baseball-archive/statistics/) has versions in Access, SQL (I'm assuming MS SQL), and comma delimited. I would rather work with the SQL version since it seems more likely to be used by developers, but I have had trouble using it. What is your opinion?

[jalpesh_007]

I think you can also go with comma delimited.

[Taywin]

OK, you are now talking about extending your project. The simpliest way to do is to take a CSV type of file data (with comma as delimiter) per jalpesh_007 suggestion and work with that. What you only need to do is to chop string to pieces. Then take each piece and fit it in your program data.

However, if you want to deal with SQL, you would have to either create your own parser or use ANTLR to help you do the SQL parsing. This approach may be too much for you right now. It requires some knowledges of SQL and adaptation to be able to apply tools to your program. It is a very good experience and improvement if you could accomplish.

[coolbeanbob]

I have my program linked to an Access database. I'm still trying to decide how I want to go about pulling the data into the program. I was considering using three classes for Player statistics (BattingStats, FieldingStats, and PitchingStats).

On the one hand, I like the three seperate classes because it seems to be more clean. For my implementation, I planned to access the database once in each class, but wouldn't this be a bad idea in terms of performance? Shouldn't I try to minimize my access to the database since it will probably cause a page fault each time?

My alternative approach is to have one class for all Player statistics, or possibly one class with batting and fielding stats, then extend this class for pitching stats if the player is a pitcher.

import java.sql.*;

public class Test {



    Test(){

    }


    static public void tryTest(){
    try {
            String p[] = new String[16];
            Class.forName("sun.jdbc.odbc.JdbcOdbcDriver");
            String dataSourceName = "lahman591";
            String dbURL = "jdbc:odbc:" + dataSourceName;
            Connection con = DriverManager.getConnection(dbURL, "", "");

            // try and create a java.sql.Statement so we can run queries
            Statement s = con.createStatement();
            s.execute("SELECT playerID, yearID, teamID, AB, H, [2B], [3B], HR, SB, CS, BB, SO, IBB, " +
                    "HBP, SH, SF FROM Batting WHERE yearID > 2010 AND teamID='CIN'"); // select the data from the table
            ResultSet rs = s.getResultSet(); // get any ResultSet that came from our query
            if (rs != null) // if rs == null, then there is no ResultSet to view
                while (rs.next()) // this will step through our data row-by-row
                {
                    for(int i = 1; i < 17; i++){ //gets player stats from Batting Table in the form of string
                        p[i-1] = rs.getString(i);   //puts stats in String array
                    }
                    /*System.out.println("Data from column_name: "
                            + rs.getString(1) + " " + rs.getString(2));*/
                }
            //s.execute("drop table TEST12345");
            s.close(); // close the Statement to let the database know we're done with it
            con.close(); // close the Connection to let the database know we're done with it
        } catch (Exception err) {
            System.out.println("ERROR: " + err);
        }
    }
}

Below is an example how the PlayerStats will look. I thought about using arrays to store the stats for each table, but I think it would be hard to keep track which index relates to which variable.

public class PlayerStats {

    //Master Table
    private String nameFirst;
    private String nameLast;
    private char bats;
    private char throwingHand;

    //Batting Table
    private String playerID;
    private int year;
    private String teamID;
    private int ab;         //At Bats
    private int hits;       //Total Hits
    private int doubles;
    private int triples;    
    private int hr;         //Home Runs
    private int sb;         //Stolen Bases
    private int cs;         //Caught Stealing
    private int bb;         //Walk
    private int so;         //Strike Outs
    private int ibb;        //Intentional BB
    private int hpb;        //Hit by Pitch
    private int sh;         //Sacrifice Hits
    private int sf;         //Sacrifice Flies
    private int gidp;       //Grounded into double play

    //Fielding Table
    private String position;

    //Pitching Table
    private int ipOuts;         //Outs Pitched (innings pitched x 3)
    private int hitsPitching;   //Hits
    private int er;             //Earned Runs
    private int hrPitching;     //Homeruns
    private int bbPitching;     //Walks
    private int soPitching;     //Strikeouts
    private int baOpp;          //Opponent's Batting Average
    private int hbpPitching;    //Batters Hit By Pitch
    private int bfp;            //Batters faced by Pitcher
    private int gidpPitching;   //Grounded into double plays by opposing batter 

    //Derived variables
    private int singles;
    private int onBase;     // hits + bb + ibb + hbp



    PlayerStats(String team){


    }

[Taywin]

This project gets more interested and complicated. There are 2 questions to be answered, I guess -- how to deal with SQL call and Player stats.

The answer to SQL call is uncertain. There are so many factors needed to be made prior to find a solution. How big the database is going to be now? How would it be changed to in the future? Shrink? Expand? Expand a lot? So what you are determining right now could be used only for right now. Do you plan to use it in the future? Do you want to leave some rooms for improvement?

Also, there are advantages and disadvatanges to have all player data in the memory. So how is your program going to be? Multiple access in a short period of time for 24/7? Access once in a couple hour?

As long as you have not made your decision, there is no way to find a suitable solution for this question.

Speaking of Player stats, there are a couple ideas that I can think of. One is to use inheritance for player. You could keep Player as parent class. Then extend it to other class with specific position. The other idea is to break the stat out to 3 different classes (that hold the stat values). Then in the player class, you create instances of those stat class depending on the player. It is very rare for a batter to have pitcher stats. I believe in one league (can't remember American or National), a pitcher won't be batting so there shouldn't be batting stats for the pitcher.

So the current player class seems to be a bit too bloated now. You may need to think about breaking it down.

[JamesCherrill]

The easiest way to to decide on classes is to go back to the English description of the domain. Being English I have no idea about baseball, but if Stats are things that a Player has, then the obvious implementation is that Stats are a class and a Player has an instance of Stats as part of its attributes. I can't comment on whether thats one Stats class or 3 classes, or subclasses because I don't know how they work.

Shouldn't I try to minimize my access to the database since it will probably cause a page fault each time?

This is far far far too early to be talking about page faults! The time to optimise Java is when the functionality is basically complete and you can benchmark different solutions to any problems that actually arise. There are few developers experienced enough to identify real-life performance problems before the code is running.
What you should do is to ensure that your classes encapsulate the domain data fully, so you can adjust or change the data storage/buffering etc entirely within the class, without affecting in any way their public interface.
(Yes, maybe I'm overstating the case, but that's better then premature optimisation)