Is this correct java? Attributes and constructors especially

Question

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

• Bobby: 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.
• psoares: 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
• Bobby: That's what I meant with 'localized', read localized to your native language which is not English.
• psoares: I think everything or almost everything is translated now
• Mike Nakis: 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

Answer 1

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.)

• psoares: 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?
• palacsint: List is an interface and ArrayList is an implementation of the List interface. javabeat.net/qna/9-difference-between-list-and-arraylist-

Answer 2

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)

Answer 3

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.

• psoares: thank you @Aaron for your answer. actually this is something I don't quite understand. What is the job of attributes here?
• Aaron J Lang: What is the job of the attributes? I'm not sure, you wrote it. What job did you intend them to serve?
• psoares: 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
• Aaron J Lang: Yep, that looks fine