5
\$\begingroup\$

I was given the task of batch process a single-level control break program to produce a lecturer information report by lecturer from a university course file.

I have attempted to create a solution using a OO design in Java. I have run into some problems as I don't think the the modules are cohesive as they are not very independent enough which makes it difficult to maintain. Also if the input file had additional lecturer's the code would have to be changed in many places to accommodate this.

I would very much appreciate it if someone would look through my design solution and offer alternatives to address the problems I've highlighted above. Also maybe suggest an alternative solution!! I'm first year student so new to programming.

Question details:

Each record on the Teaching-module file contains details of a lecture’s teaching load; that is, a record number, the school code, the Lecturers department number, the lecturer’s identity number, the lecture’s name, the module number of the module being taught, the credit hours for that module, and class size. There may be more than one record for each lecturer, depending on the number of modules he or she teaches. The Teaching-module file must be sorted into ascending sequence of lecturer number within department, within school. Tip treat as a concatenated single field.

The program is to read the sorted teaching-module file sequentially, calculate the lecture’s contact hours ((class size/50)* credit hours), and produce the lecturer information report.

Report example:

enter image description here

Below is my solution so far:

    public class DSAPDAssignmentB2 {

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) {

        String teachMod =  ("1,CSM,501,SM0156,Simon Thorne,BCO200,24,30,"+
                            "2,CSM,500,AC1157,Nigel Jones,BCO104,24,60,"+
                            "3,CSM,500,SM0156,Simon Thorne,BCO113,12,60,"+
                            "4,CSM,500,AC1157,Nigel Jones,BCO104,24,30,"+
                            "5,CSM,500,AC1156,Richard Adlam,BCO113,12,60,"+
                            "6,CSM,500,AC1157,Nigel Jones,BCO109,12,90,"+
                            "7,CSM,503,SM0156,Simon Thorne,CIS414,12,30,"+
                            "8,CSM,501,AC1156,Richard Adlam,BCO222,12,40,"+
                            "9,CSM,500,AC1156,Richard Adlam,BCO114,12,60,"+
                            "10,CSM,501,AC1157,Nigel Jones,BCO200,60,40,"+
                            "11,CSM,500,AC1156,Richard Adlam,BCO106,24,60,"+
                            "12,CSM,501,SM0156,Simon Thorne,BCO207,12,30,"+
                            "13,CSM,500,AC1157,Nigel Jones,BCO112,24,30,");
        //LecturerDetails simonThorne = new LecturerDetails("Simon Thorne", "SM0156");
        ArrayList<String> aList = fileToRecords(teachMod, 8);
        aList = trimOffRecNo(aList);
        aList = sortRecords(aList,4,7);
        updateLecturerFile(aList);
        createReport();
        //System.out.println("Simon Thorne details are: ");
        //simonThorne.toString();

        //System.out.println("Nigel Jones details are: ");
        //nigelJones.toString();
        //System.out.println("Richard Adlam details are: ");
        //richardAdlam.toString();
        //System.out.println("*************************************1");
        //for(String string : aList){
        //    System.out.println(string);
        //}
        System.out.println("End");
    }//End main

    public static LecturerDetails simonThorne = new LecturerDetails("Simon Thorne", "SM0156");
    public static LecturerDetails nigelJones = new LecturerDetails("Nigel Jones", "AC1157");
    public static LecturerDetails richardAdlam = new LecturerDetails("Richard Adlam", "AC1156");


    //Takes a String and splits it into chunks, each chunk is stored in an ArrayList.
    //Here we specify how much of the String we chunk into our list.
    //Each chunk in the ArrayList represents a lecturer record taken from the teaching module
    public static ArrayList fileToRecords(String s, int chunk){
        ArrayList<String> chunks = new ArrayList<>();       //Create our ArrayList
        StringBuffer sb = new StringBuffer();               //We use a StringBuffer to append each character
        char[] c = s.toCharArray();                         //Convert our String into a char array
        int j = 0;                                              
        for(int i = 0; i < s.length(); i++){                //Loop through each char in our String
            sb.append(c[i]);                                //Append each char as we take a stroll
            if(c[i]==','){                                  //Check if the char is a delimiter
                j++;                                        //If so then keep a count as we walk past
                if(j==chunk){                               //Check if we have reached the end marker for our chunk
                    chunks.add(sb.toString());              //If so then add the chunk to our ArrayList
                    j=0;                                    //reset delimiter count
                    sb = new StringBuffer();                //Empty our StringBuffer
                }
            }
        }
        return chunks;                                      //return array
    }

    //Here we trim off the record numbers as this is useless information
    //The record number is followed by a delimiter so we eject both
    public static ArrayList trimOffRecNo(ArrayList<String> list){
        ArrayList<String> aList = new ArrayList<>();        //Create our ArrayList
        for(int i=0;i<list.size();i++){                     //Loop through each element in list
            for(int j=0; j<list.get(i).length();j++){       //Loop through each char
                if(list.get(i).charAt(j) == ','){           //Check for first delimiter...
                    //...then whip out the rest of the String
                    aList.add(list.get(i).substring(j+1, list.get(i).length()));
                    break;                                  //Let's have no exceptions here
                }
            }
        }
        return aList;                                       //Return array 
    }

    public static void updateLecturerFile(ArrayList<String> list){
        ArrayList<String> aList = new ArrayList<>();
        aList = sortRecords(list, 9,14);
        String temp = null;

        for(int i = 0; i < list.size();i++){
            temp = aList.get(i).substring(8,14);

            switch(temp){
                case "AC1156":
                    richardAdlam.updateMe(aList.get(i).toString());
                    System.out.println(aList.get(i).toString());
                    break;
                case "AC1157":
                    nigelJones.updateMe(aList.get(i).toString());
                    break;
                case "SM0156":
                    simonThorne.updateMe(aList.get(i).toString());
                    //System.out.println(aList.get(i).toString());
                    break;
            }
        }


    }
    public static void createReport(){
        String temp = null;
        System.out.println("\nSCHOOL\tDEPARTMENT\tLECTURER\tLECTURER\tMODULE\tCREDIT\tCLASS\tCONTACT");
        System.out.println("ID\tNO\t\tID\t\tNAME\t\tCODE\tHOURS\tSIZE\tHOURS");
        ArrayList<TeachingLoad> simon = simonThorne.getTeachingLoad();
        ArrayList<TeachingLoad> nigel = nigelJones.getTeachingLoad();
        ArrayList<TeachingLoad> richard = richardAdlam.getTeachingLoad();
        String dept = "500";
        int deptHours = 0;
        int schoolHours = 0;
        for(int i = 0; i<3;i++){
            if(i==1)
                dept = "501";
            else if(i==2)
                dept = "503";
            int hours = 0;
            int totalHours = 0;

            for(TeachingLoad tl : simon){

                if(tl.getDepartmentNumber().equals(dept)){
                    hours = (int) (hours + (double)Integer.parseInt(tl.getClassSize())/50*Integer.parseInt(tl.getCreditHours()));
                    System.out.println(tl.getSchoolCode()+"\t"+tl.getDepartmentNumber()+"\t\t"+
                            simonThorne.getID()+"\t\t"+simonThorne.getName()+"\t"+
                            tl.getModuleNo()+"\t"+tl.getCreditHours()+"\t"+tl.getClassSize()+"\t"+hours);
                    totalHours = totalHours + hours;

                }
            }
            deptHours = deptHours + totalHours;
            System.out.println("\t\t\tCONTACT HOURS FOR LECTURER \t\t\t\t"+totalHours);
            hours = 0;
            totalHours = 0;
            for(TeachingLoad tl : nigel){
                if(tl.getDepartmentNumber().equals(dept)){
                    hours = (int) (hours + (double)Integer.parseInt(tl.getClassSize())/50*Integer.parseInt(tl.getCreditHours()));
                    System.out.println(tl.getSchoolCode()+"\t"+tl.getDepartmentNumber()+"\t\t"+
                            nigelJones.getID()+"\t\t"+nigelJones.getName()+"\t"+
                            tl.getModuleNo()+"\t"+tl.getCreditHours()+"\t"+tl.getClassSize()+"\t"+hours);
                    totalHours = totalHours + hours;
                }

            }
            deptHours = deptHours + totalHours;
            System.out.println("\t\t\tCONTACT HOURS FOR LECTURER \t\t\t\t"+totalHours);
            hours = 0;
            totalHours = 0;
            for(TeachingLoad tl : richard){
                if(tl.getDepartmentNumber().equals(dept)){
                    hours = (int) (hours + (double)Integer.parseInt(tl.getClassSize())/50*Integer.parseInt(tl.getCreditHours()));
                    System.out.println(tl.getSchoolCode()+"\t"+tl.getDepartmentNumber()+"\t\t"+
                            richardAdlam.getID()+"\t\t"+richardAdlam.getName()+"\t"+
                            tl.getModuleNo()+"\t"+tl.getCreditHours()+"\t"+tl.getClassSize()+"\t"+hours);
                    totalHours = totalHours + hours;
                }
            }

            deptHours = deptHours + totalHours;
            schoolHours = schoolHours +deptHours;
            System.out.println("\t\t\tCONTACT HOURS FOR LECTURER \t\t\t\t"+totalHours);
            System.out.println("\tCONTACT HOURS FOR DEPARTMENT "+deptHours);
            deptHours = 0;
        }

        System.out.println("CONTACT HOURS FOR SCHOOL "+schoolHours);


    }


    //Sorts the ArrayList records naturally. User can specify start & end of chunk in String to sort.
    //Could be useful if we wanted to sort by module number or name!
    public static ArrayList sortRecords(ArrayList<String> list, int start, int end){
        String s1, s2, temp;                                //Some local variables
        for(int i = 0 ; i < list.size(); i++){              //Loop throuth each element in list
            for(int j = list.size()-1; j>i  ;j--){          //Loop through each char
                s1 = list.get(j-1).substring(start,end);    //Exreact 1st substring
                System.out.println(s1);
                s2 = list.get(j).substring(start,end);      //Extract 2nd substring
                if(i+1<list.size()&&s1.compareTo(s2)>-1){   //Compare them lexicographically
                    temp = list.get(j-1);
                    //If s1 follows s2 then switch both
                    list.set(j-1, list.get(j));
                    list.set(j, temp);
                }
            }
        }
        for(String s : list){
            System.out.println(s);
        }
        return list;                                        //Retrun sorted list
    }//End sortRecords

}//End class

    public class LecturerDetails {

    private String ID;
    private String name;
    ArrayList<TeachingLoad> list;

    public LecturerDetails() {
    }

    public LecturerDetails(String name, String ID) {
        this.name = name;
        this.ID = ID;
        list = new ArrayList<>(20);
    }

    public void updateMe(String file){
        String school=null, deptNo=null, moduleNo=null, creditHours=null, classSize=null;
        String temp;
        int delimitCount = 0;
        int lastCount = 0;
        for(int i = 0; i < file.length(); i++){
            if(file. charAt(i) == ','){
                temp = file.substring(lastCount, i);
                lastCount = i+1;
                delimitCount++;
                switch(delimitCount){
                    case 1:
                        school = temp;
                        break;
                    case 2:
                        deptNo = temp;
                        break;
                    case 5:
                        moduleNo = temp;
                        break;
                    case 6:
                        creditHours = temp;
                        break;
                    case 7:
                        classSize = temp;
                        break;
                }
            }
        }
        list.add(new TeachingLoad(school, deptNo, moduleNo, creditHours, classSize));
    }

    public String getID() {
        return ID;
    }

    public String getName() {
        return name;
    }

    public void setID(String ID) {
        this.ID = ID;
    }

    public void setName(String name) {
        this.name = name;
    }

    public ArrayList<TeachingLoad> getTeachingLoad() {
        return list;
    }

    @Override
    public String toString() {
        //for(TeachingLoad tl : list){
        //    System.out.println(tl);
        //}
        return ("LecturerDetails");// + "ID=" + ID + ", name=" + name);
    } 
}

    public class TeachingLoad {
    private String schoolCode;
    private String departmentNumber;
    private String moduleNo;
    private String creditHours;
    private String classSize;

    public TeachingLoad() {
    }

    public TeachingLoad(String schoolCode, String departmentNumber, String moduleNo, String creditHours, String classSize) {
        this.schoolCode = schoolCode;
        this.departmentNumber = departmentNumber;
        this.moduleNo = moduleNo;
        this.creditHours = creditHours;
        this.classSize = classSize;
    }

    public String getSchoolCode() {
        return schoolCode;
    }

    public String getDepartmentNumber(){
        return departmentNumber;
    }

    public String getModuleNo() {
        return moduleNo;
    }

    public String getCreditHours() {
        return creditHours;
    }

    public String getClassSize() {
        return classSize;
    }

    public void setSchoolCode(String schoolCode) {
        this.schoolCode = schoolCode;
    }

    public void setDepartmentNumber(String departmentNumber){
        this.departmentNumber = departmentNumber;
    }

    public void setModuleNo(String moduleNo) {
        this.moduleNo = moduleNo;
    }

    public void setCreditHours(String creditHours) {
        this.creditHours = creditHours;
    }

    public void setClassSize(String classSize) {
        this.classSize = classSize;
    }

    @Override
    public String toString() {
        return "Entry{" + "schoolCode=" + schoolCode + " moduleNo=" + moduleNo + " creditHours=" + creditHours + " classSize=" + classSize + '}';
    }
}

Below is a printout:

enter image description here

I suppose it's a bit of an ask...I've changed the main line (see below) Here I've now created a Map which contains details of lecturers. The call to create lecturerObject() searches for the new lecturer unique ID's and creates new objects if they don't already exist. So now in updateLecturerFile() I have been able to remove the switch statement and the use of "Magic Numbers" - Thanks to UnholySampler for pointing that out to me! I suppose this way the program can handle reports of different numbers of lecturers now (great I've made some progress here). The lecturer unique ID's are "SM0156" & "AC1157" etc etc. I will have to amend the createReport() function to accommodate these changes. Within the createReport() function it still relies on hard coding to process the 3 lecturer details within the input string. Now I want it to be able to handle any number of lecturers so I will have to remove the three Collection loops there now that I have the lecturer details stores in a Map this should be doable.??? Still though I feel the program could be better as I mentioned above. The updated main line is below: (I'll post the updated createReport() when I know what I got to do with it!)

public class DSAPDAssignmentB2 {

/**
 * @param args the command line arguments
 */
public static void main(String[] args) {

    String teachMod =  ("1,CSM,501,SM0156,Simon Thorne,BCO200,24,30,"+
                        "2,CSM,500,AC1157,Nigel Jones,BCO104,24,60,"+
                        "3,CSM,500,SM0156,Simon Thorne,BCO113,12,60,"+
                        "4,CSM,500,AC1157,Nigel Jones,BCO104,24,30,"+
                        "5,CSM,500,AC1156,Richard Adlam,BCO113,12,60,"+
                        "6,CSM,500,AC1157,Nigel Jones,BCO109,12,90,"+
                        "7,CSM,503,SM0156,Simon Thorne,CIS414,12,30,"+
                        "8,CSM,501,AC1156,Richard Adlam,BCO222,12,40,"+
                        "9,CSM,500,AC1156,Richard Adlam,BCO114,12,60,"+
                        "10,CSM,501,AC1157,Nigel Jones,BCO200,60,40,"+
                        "11,CSM,500,AC1156,Richard Adlam,BCO106,24,60,"+
                        "12,CSM,501,SM0156,Simon Thorne,BCO207,12,30,"+
                        "13,CSM,500,AC1157,Nigel Jones,BCO112,24,30,");


    ArrayList<String> aList = fileToRecords(teachMod, 8);
    createLecturerObject(aList);  //Create a collection of LecturerDetails in a Map
    aList = trimOffRecNo(aList);
    //aList = sortRecords(aList,4,7);
    updateLecturerFile(aList);
    //createReport();

}//End main


public static HashMap<String, LecturerDetails> lecturerList = new HashMap();

//Takes a String and splits it into chunks, each chunk is stored in an ArrayList.
//Here we specify how much of the String we chunk into our list.
//Each chunk in the ArrayList represents a lecturer record taken from the teaching module
public static ArrayList fileToRecords(String s, int chunk){
    ArrayList<String> chunks = new ArrayList<>();       //Create our ArrayList
    StringBuffer sb = new StringBuffer();               //We use a StringBuffer to append each character
    char[] c = s.toCharArray();                         //Convert our String into a char array
    int j = 0;                                              
    for(int i = 0; i < s.length(); i++){                //Loop through each char in our String
        sb.append(c[i]);                                //Append each char as we take a stroll
        if(c[i]==','){                                  //Check if the char is a delimiter
            j++;                                        //If so then keep a count as we walk past
            if(j==chunk){                               //Check if we have reached the end marker for our chunk
                chunks.add(sb.toString());              //If so then add the chunk to our ArrayList
                j=0;                                    //reset delimiter count
                sb = new StringBuffer();                //Empty our StringBuffer
            }
        }
    }
    return chunks;                                      //return array
}


public static void createLecturerObject(ArrayList<String> al){
    String thisLec = null;
    int delimitCount = 0;
    for(int i = 0; i<al.size();i++){
        for(int j = 0;j<al.get(i).length();j++){
            if(al.get(i).charAt(j) == ','){
                delimitCount++;
                if(delimitCount == 3){
                   thisLec = al.get(i).substring(j+1, j+7);
                   if(!lecturerList.containsKey(thisLec)){
                       lecturerList.put(thisLec, new LecturerDetails());
                       lecturerList.get(thisLec).setID(thisLec);
                   }
                   delimitCount = 0;
                   break;
                }
            }
        }
    }
}

//Here we trim off the record numbers as this is useless information
//The record number is followed by a delimiter so we eject both
public static ArrayList trimOffRecNo(ArrayList<String> list){
    ArrayList<String> aList = new ArrayList<>();        //Create our ArrayList
    for(int i=0;i<list.size();i++){                     //Loop through each element in list
        for(int j=0; j<list.get(i).length();j++){       //Loop through each char
            if(list.get(i).charAt(j) == ','){           //Check for first delimiter...
                //...then whip out the rest of the String
                aList.add(list.get(i).substring(j+1, list.get(i).length()));
                break;                                  //Let's have no exceptions here
            }
        }
    }
    return aList;                                       //Return array 
}

public static void updateLecturerFile(ArrayList<String> list){
    ArrayList<String> aList = new ArrayList<>();
    //aList = sortRecords(list, 9,14);
    aList = list;
    String temp = null;
    for(int i = 0; i < aList.size();i++){
        temp = aList.get(i).substring(8,14);
        for(String key : lecturerList.keySet()){
            if(key.equals(temp)){
                lecturerList.get(temp).updateMe(aList.get(i));
                System.out.println("In update lecturer");
            }
        }
    }
}
/**
public static void createReport(){
    String temp = null;
    System.out.println("\nSCHOOL\tDEPARTMENT\tLECTURER\tLECTURER\tMODULE\tCREDIT\tCLASS\tCONTACT");
    System.out.println("ID\tNO\t\tID\t\tNAME\t\tCODE\tHOURS\tSIZE\tHOURS");
    ArrayList<TeachingLoad> simon = simonThorne.getTeachingLoad();
    ArrayList<TeachingLoad> nigel = nigelJones.getTeachingLoad();
    ArrayList<TeachingLoad> richard = richardAdlam.getTeachingLoad();
    String dept = "500";
    int deptHours = 0;
    int schoolHours = 0;
    for(int i = 0; i<3;i++){
        if(i==1)
            dept = "501";
        else if(i==2)
            dept = "503";
        int hours = 0;
        int totalHours = 0;

        for(TeachingLoad tl : simon){

            if(tl.getDepartmentNumber().equals(dept)){
                hours = (int) (hours + (double)Integer.parseInt(tl.getClassSize())/50*Integer.parseInt(tl.getCreditHours()));
                System.out.println(tl.getSchoolCode()+"\t"+tl.getDepartmentNumber()+"\t\t"+
                        simonThorne.getID()+"\t\t"+simonThorne.getName()+"\t"+
                        tl.getModuleNo()+"\t"+tl.getCreditHours()+"\t"+tl.getClassSize()+"\t"+hours);
                totalHours = totalHours + hours;

            }
        }
        deptHours = deptHours + totalHours;
        System.out.println("\t\t\tCONTACT HOURS FOR LECTURER \t\t\t\t"+totalHours);
        hours = 0;
        totalHours = 0;
        for(TeachingLoad tl : nigel){
            if(tl.getDepartmentNumber().equals(dept)){
                hours = (int) (hours + (double)Integer.parseInt(tl.getClassSize())/50*Integer.parseInt(tl.getCreditHours()));
                System.out.println(tl.getSchoolCode()+"\t"+tl.getDepartmentNumber()+"\t\t"+
                        nigelJones.getID()+"\t\t"+nigelJones.getName()+"\t"+
                        tl.getModuleNo()+"\t"+tl.getCreditHours()+"\t"+tl.getClassSize()+"\t"+hours);
                totalHours = totalHours + hours;
            }

        }
        deptHours = deptHours + totalHours;
        System.out.println("\t\t\tCONTACT HOURS FOR LECTURER \t\t\t\t"+totalHours);
        hours = 0;
        totalHours = 0;
        for(TeachingLoad tl : richard){
            if(tl.getDepartmentNumber().equals(dept)){
                hours = (int) (hours + (double)Integer.parseInt(tl.getClassSize())/50*Integer.parseInt(tl.getCreditHours()));
                System.out.println(tl.getSchoolCode()+"\t"+tl.getDepartmentNumber()+"\t\t"+
                        richardAdlam.getID()+"\t\t"+richardAdlam.getName()+"\t"+
                        tl.getModuleNo()+"\t"+tl.getCreditHours()+"\t"+tl.getClassSize()+"\t"+hours);
                totalHours = totalHours + hours;
            }
        }

        deptHours = deptHours + totalHours;
        schoolHours = schoolHours +deptHours;
        System.out.println("\t\t\tCONTACT HOURS FOR LECTURER \t\t\t\t"+totalHours);
        System.out.println("\tCONTACT HOURS FOR DEPARTMENT "+deptHours);
        deptHours = 0;
    }

    System.out.println("CONTACT HOURS FOR SCHOOL "+schoolHours);

}*/


//Sorts the ArrayList records naturally. User can specify start & end of chunk in String to sort.
//Could be useful if we wanted to sort by module number or name!
public static ArrayList sortRecords(ArrayList<String> list, int start, int end){
    String s1, s2, temp;                                //Some local variables
    for(int i = 0 ; i < list.size(); i++){              //Loop throuth each element in list
        for(int j = list.size()-1; j>i  ;j--){          //Loop through each char
            s1 = list.get(j-1).substring(start,end);    //Exreact 1st substring
            //System.out.println(s1);
            s2 = list.get(j).substring(start,end);      //Extract 2nd substring
            if(i+1<list.size()&&s1.compareTo(s2)>-1){   //Compare them lexicographically
                temp = list.get(j-1);
                //If s1 follows s2 then switch both
                list.set(j-1, list.get(j));
                list.set(j, temp);
            }
        }
    }
    return list;                                        //Retrun sorted list
}//End sortRecords

}//End clasS
\$\endgroup\$
5
\$\begingroup\$

Variable Names: The name should describe the data that is being stored there. list and aList don't give you any context for what they are being used for. Try to avoid single letter names or abbreviations that might not be clear to other people reading the code.

Additionally, when I saw

for(TeachingLoad tl : simon){

I was confused because I expected simon to be an object that represented a person, not a list of things that are associated with the person. In this case, you only use these lists to iterate over, so you could change it to

for(TeachingLoad tl : simon.getTeachingLoad()){

and now there is one less variable to worry about.


For trimOffRecNo(), you only ever use i to get the current element from list (which you do repeatedly). You can just change that to be a foreach loop. This will make the code more readable.

public static ArrayList trimOffRecNo(ArrayList<String> list){
    ArrayList<String> aList = new ArrayList<>();        //Create our ArrayList
    for(String rec : list){                     //Loop through each element in list
        for(int j=0; j<rec.length();j++){       //Loop through each char
            if(rec.charAt(j) == ','){           //Check for first delimiter...
                //...then whip out the rest of the String
                aList.add(rec.substring(j+1, rec.length()));
                break;                                  //Let's have no exceptions here
            }
        }
    }
    return aList;                                       //Return array 
}

Additionally, you can write this code without the inner loop.

public static List<String> trimOffRecNo(List<String> list){
  List<String> result = new ArrayList<>();
  for(String rec : list){
    int index = rec.indexOf(',');
    if (index == -1) {
      //TODO: decide what to do when there is no comma
    } else {
      result.add(rec.substring(index+1, rec.length());
    }
  }
  return result;
}

I would change the function name. The current name seems to imply that you are changing the content you are passing in. Instead, what is happening is that you are extracting values.


Repeated code: The best example of this is createReport(). This could be rewritten along these lines:

public static int printLoad(LecturerDetails details, String dept) {
  int totalHours = 0;

  for (TeachingLoad tl : details.getTeachingLoad()) {
    //calculate hours, add to totalHours, and print content
  }
  return totalHours;
}

public static void createReport() {
  //setup stuff

  int deptHours = 0;

  for (LecturerDetails details : list of professors) {
    deptHours += printLoad(details, getDept(details));
    // between detail stuff
  }
  // end stuff
}

When you are adding up hours, I suspect there might be a bug.

for each item {
    hours = hours + mathForThisItem();
    // other things
    totalHours = totalHours + hours;
}

hours is accumulating with each item in addition to totalHours. I don't know the business logic behind this math, but I suspect that hours is intended to be the hours for just one item.


You can split a String with split() instead of having to iterate over a string's characters to match on a specific. Since you weren't using indexOf() in another point, it might make sense to look over the other methods that are available in the built in classes that come with Java.


updateLecturerFile() has a number of cases where you have magic numbers or values. The link gives a simple example of what I'm talking about and describing why this is not a great idea.

\$\endgroup\$
  • \$\begingroup\$ Thanks, I've just added a workaround to the 'Magic Number' issue. See updated main line above... \$\endgroup\$ – TripVoltage Nov 18 '13 at 16:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.