Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm learning java, although because of work I didn't had much time to go to classes. We had a final work to do but since I'm more familiarised with python I'm not sure if I'm doing java correctly...

I'm also a bit confused about attributes and constructors, I don't really understand the use of it.

For this work we have to do a server class and a client class. We have 4 files, one with times (in seconds) and points for bike female and male, and others with times and points run female and male. We then have a times file where each athlete have the time (minutes) for bike and run. We need to calculate the points for each time with linear interpolation, and then sort then, to see which athlete was the best one.

Where's what I've done at the server class:

import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.util.ArrayList;
import java.util.Scanner;

public class Scorer {

private String bike;
private String run;
private ArrayList<ArrayList<Integer>> athletes;
private boolean gender;

public Scorer(String bikeF, String runF, ArrayList<ArrayList<Integer>> athletes) {
    this.bike = bikeF;
    this.run = runF;
    this.athletes = athletes;
 }

public Scorer(ArrayList<ArrayList<Integer>> athletes, boolean gender) {
    this.athletes = athletes;
    this.gender = gender;
    if (gender == true) {
        this.bike = bike + "F.tab";
        this.run = run + "F.tab";
    } else {    
        this.bike = bike + "M.tab";
        this.run = run + "M.tab";
    }

}

public int[][] valsProximos(String table, ArrayList<ArrayList<Integer>> athletes, int n)
    throws FileNotFoundException {
            // compare file times and points with array to find distances and points closest to calculate linear interpolation
           Scanner tables = new Scanner (new FileReader(table));

    int [][] tabPoints = new int [9][2]; 
            // this case, each column has a meaning, 1 athlete 2 points (if equals) 3 athlete 4 difference between times 5 max time 6 max points 7 athlete 8 difference between times 9 min time 10 min points      
    int [][] values = new int [athletes.size()][10];

    for (int i=0; i<tabPoints.length;i++) {
        for (int j =0;j<tabPoints[0].length;j++)
            tabPoints[i][j]= tables.nextInt();
    }

    for (int i=0; i<athletes.size(); i++) {
        for (int j=0; j<tabPoints.length; j++) {
            if (athletes.get(i).get(n) == tabPoints[j][0]) {
                values[i][0] = athletes.get(i).get(0);
                values[i][1] = tabPoints[j][1];
            } else {

                if (tabPoints[j][0] > athletes.get(i).get(n)) {
                    // calculate difference between each time and the time in the table
                    int dif = tabPoints[j][0] - athletes.get(i).get(n);

                    if (values[i][2] != athletes.get(i).get(0)) {
                        values[i][2] = athletes.get(i).get(0);
                        values[i][3] = dif;
                        values[i][4] = tabPoints[j][0]; //maxTime
                        values[i][5] = tabPoints[j][1]; // maxPoint
                    } else if (dif < values[i][3]) {
                            values[i][3] = dif;
                            values[i][4] = tabPoints[j][0];
                            values[i][5] = tabPoints[j][1];
                    } 
                } else { 
                    int dif1 = athletes.get(i).get(n) - tabPoints[j][0];
                    if (values[i][6] != athletes.get(i).get(0)) {
                        values[i][6] = athletes.get(i).get(0);
                        values[i][7] = dif1;
                        values[i][8] = tabPoints[j][0]; // minTime
                        values[i][9] = tabPoints[j][1]; // minPoint
                    } else {
                        if (dif1 < values[i][7]) {
                            values[i][7] = dif1;
                            values[i][8] = tabPoints[j][0];
                            values[i][9] = tabPoints[j][1];
                        }
                    }
                }   
            }   
        }
    }
    return values;
}

public double intLinear(int maxTime, int time, int minTime, 
        int maxPoint, int minPoint) {
    // calculate points given time acRunding to linear interpolation
    double intLinear = (double)(maxTime - time)/(maxTime - minTime) 
        * minPoint + (double)(time - minTime)/(maxTime - minTime) * maxPoint;
    // round to closest number
    double athletePoint = (double)Math.round(intLinear);
    return athletePoint;
}

public int[][] Score(int [][] valBike, int [][] valRun, 
        ArrayList<ArrayList<Integer>> athletes) {

    int [][] punctuate = new int [athletes.size()][4];
    for (int i=0; i<valBike.length; i++) {

        if (athletes.get(i).get(0) == valBike[i][0]){
            punctuate[i][0] = athletes.get(i).get(0);
            punctuate[i][1] = valBike[i][1];
        } else {
            int maxTime = valBike[i][4];
            int time = athletes.get(i).get(1);
            int minTime = valBike[i][8];
            int maxPoint = valBike[i][5];
            int minPoint = valBike[i][9];
            double athletePoint = intLinear(maxTime, time, minTime, maxPoint, minPoint);
            punctuate[i][0] = athletes.get(i).get(0);
            punctuate[i][1] = (int) athletePoint;
        }

        if (athletes.get(i).get(0) == valRun[i][0]){
            // Verify that we are inserting points at right position
            //if (punctuate[i][0] == valRun[i][0]) {
                punctuate[i][2] = valRun[i][1];
            //} 
        } else {
            int maxTime = valRun[i][4];
            int time = athletes.get(i).get(2);
            int minTime = valRun[i][8];
            int maxPoint = valRun[i][5];
            int minPoint = valRun[i][9];
            double athletePoint = intLinear(maxTime, time, minTime, maxPoint, minPoint);
            //if (punctuate[i][0] == valRun[i][2]) {
                punctuate[i][2] = (int) athletePoint;
            //} 
        }
    }
    for (int i=0; i<punctuate.length; i++) {
        // total points
        punctuate[i][3] = punctuate[i][1] + punctuate[i][2];
    }
return punctuate;
}

public int[][] ScoreF(int [][] order, int colNum) {
    for (int row=0; row< order.length; row++){
        for (int row2=row+1; row2<order.length; row2++){
            // modify acRunding to the column we want to sort
            if(order[row][colNum]<order[row2][colNum]){
                for(int column=0; column<order[0].length; column++) {   
                    int temp = order[row][column];
                    order[row][column] = order[row2][column];
                    order[row2][column] = temp;
                }
            }
        }
    }
    return order;
}

}

and the client class:

 import java.io.BufferedWriter;
 import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Scanner;

 public class DualthonProof {

/**
 * @param args
 * @throws IOException 
 */
public static void main(String[] args) throws IOException {
    // ask user name of file
    Scanner input= new Scanner(System.in);
    System.out.println("Enter file name bike female:");
    String bikeF = input.nextLine();
    System.out.println("Enter file name female run:");
    String runF = input.nextLine();
    System.out.println("Enter file name male bike:");
    String bikeM = input.nextLine();
    System.out.println("Enter file name male run:");
    String runM = input.nextLine();

    String tabBikeF = "bikeF.tab";
    String tabRunF = "runF.tab";
    String tabBikeM = "bikeM.tab";
    String tabRunM = "runM.tab";
    // if user didn't wrote anything use file
    if (bikeF.equals("")) 
        bikeF = tabBikeF; 
    if (runF.equals(""))
        runF = tabRunF; 
    if (bikeM.equals(""))
        bikeM = tabBikeM; 
    if (runM.equals(""))
        runM = tabRunM;

    Scanner ficheiro = new Scanner (new FileReader ("times.txt"));
    // Ignore first line 
    ficheiro.nextLine();

    ArrayList<ArrayList<Integer>> athleteF = new ArrayList<ArrayList<Integer>>();
    ArrayList<ArrayList<Integer>> athleteM = new ArrayList<ArrayList<Integer>>();

    while (ficheiro.hasNextLine()) {
        String row = ficheiro.nextLine();
        String[] section = row.split("\t");

        String[] split1 = section[2].split(":");
        String[] split2 = section[3].split(":");
        // Convert string to integer
        int num1 = Integer.parseInt(split1[0]);
        int num2 = Integer.parseInt(split1[1]);
        int secsBike = num1 * 60 + num2;
        int num3 = Integer.parseInt(split2[0]);
        int num4 = Integer.parseInt(split2[1]);
        int secsRun = num3 * 60 + num4;

        if (section[1].equals("F")) {
            athleteF.add(new ArrayList<Integer>());
            athleteF.get(athleteF.size()-1).add(Integer.parseInt(section[0]));
            athleteF.get(athleteF.size()-1).add(secsBike);
            athleteF.get(athleteF.size()-1).add(secsRun);    
        } else if (section[1].equals("M")) {
            athleteM.add(new ArrayList<Integer>());
            athleteM.get(athleteM.size()-1).add(Integer.parseInt(section[0]));
            athleteM.get(athleteM.size()-1).add(secsBike);
            athleteM.get(athleteM.size()-1).add(secsRun);
        }
    }   

    Scorer ptsF = new Scorer(bikeF, runF, athleteF);
    int [][] valBikeF = ptsF.valsProximos(bikeF, athleteF, 1);
    int [][] valRunF = ptsF.valsProximos(runF, athleteF, 2);
    int [][] pointingF = ptsF.Score(valBikeF, valRunF, athleteF);
    int [][] sortedF = ptsF.ScoreF(pointingF, 3);

}
 }
share|improve this question
2  
I'm not a friend of localized variable names and comments. You might want to consider to keep everything in English, especially if you come with your code to an English website to let it be reviewed. –  Bobby Jan 2 '12 at 12:16
    
yes, you're right. Normally I program in english but since this was an assignment to school I left it in portuguese and only change the comments. Can you explain your statement about localized variables? I've came here to learn with my mistakes –  pavid Jan 2 '12 at 12:30
    
That's what I meant with 'localized', read localized to your native language which is not English. –  Bobby Jan 2 '12 at 12:31
    
I think everything or almost everything is translated now –  pavid Jan 2 '12 at 13:51
1  
pavid, use the @ sign before someone's name to make sure they receive a notification about your comment. So, you should prefix your comment with @Bobby –  Mike Nakis Jan 2 '12 at 13:55

3 Answers 3

up vote 4 down vote accepted

At first sight, the biggest problem is the double generic lists, like:

ArrayList<ArrayList<Integer>> athleteF = new ArrayList<ArrayList<Integer>>();

It's really hard to read and error-prone, since the lots of magic numbers which indexes the array. It's easy to mix-up the indexes and hard to remember which index stores which value. Use at least constants instead of the numbers.

Anyway, you should create an Athlete class which stores all data of an athlete and store Athlete objects in the list:

final List<Athlete> athleteF = new ArrayList<Athlete>();
public class Athlete {

    // TODO: set a proper name for this field
    private int someDataNeedName;
    private int secsBike;
    private int secsRun;

    public Athlete(final int someDataNeedName, final int secsBike, 
            final int secsRun) {
        this.someDataNeedName = someDataNeedName;
        this.secsBike = secsBike;
        this.secsRun = secsRun;
    }

    public int getSomeDataNeedName() {
        return someDataNeedName;
    }

    public void setSomeDataNeedName(final int someDataNeedName) {
        this.someDataNeedName = someDataNeedName;
    }

    public int getSecsBike() {
        return secsBike;
    }

    public void setSecsBike(final int secsBike) {
        this.secsBike = secsBike;
    }

    public int getSecsRun() {
        return secsRun;
    }

    public void setSecsRun(final int secsRun) {
        this.secsRun = secsRun;
    }
}

This class stores its data with names (these are the names of the fields).

Usage:

final Athlete athlete = new Athlete(Integer.parseInt(section[0]), secsBike, secsRun);

if (section[1].equals("F")) {
    athleteF.add(athlete);
} else if (section[1].equals("M")) {
    ...
}

(I hope it helps a little bit and somebody has time for a complete review.)

share|improve this answer
    
thank you @palacsint for your answer. actually at first I tried to use List (without creating a new class) but I didn't something wrong and it didn't work. That's why I moved to arrayList. What is the main difference between those two? –  pavid Jan 3 '12 at 9:05
1  
List is an interface and ArrayList is an implementation of the List interface. javabeat.net/qna/9-difference-between-list-and-arraylist- –  palacsint Jan 3 '12 at 9:36

I haven't read it all, but one thing I picked up:

public Scorer(ArrayList> athletes, boolean gender) {
    this.athletes = athletes;
    this.gender = gender;
    if (gender == true) {
        this.bike = bike + "F.tab";
        this.run = run + "F.tab";
    } else {    
        this.bike = bike + "M.tab";
        this.run = run + "M.tab";
    }

}

When this constructor is called, bike and run are both null. null + "F.tab " will either throw a nullpointer or give you "nullF.tab", neither of which I think is your intention. You propably want bike="F.tab"; run="F.tab"; etc.

For the record, because this constructor does not declare it's own bike and run variables, this.bike and bike are the same reference.

share|improve this answer
    
thank you @Aaron for your answer. actually this is something I don't quite understand. What is the job of attributes here? –  pavid Jan 3 '12 at 9:04
    
What is the job of the attributes? I'm not sure, you wrote it. What job did you intend them to serve? –  Aaron J Lang Jan 3 '12 at 13:32
    
I think I've understood. it doesn't make sense that way but if I do something like this: private String bike = "filebike.txt"; it would be more appropriate @Aaron –  pavid Jan 3 '12 at 16:34
    
Yep, that looks fine –  Aaron J Lang Jan 3 '12 at 17:45

I'd strongly suggest to use the same order for parameters in overloaded methods/constructors, this is standard and expected:

public Scorer(ArrayList<ArrayList<Integer>> athletes, String bikeF, String runF)
public Scorer(ArrayList<ArrayList<Integer>> athletes, boolean gender)
share|improve this answer

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.