Is there a more efficient way of counting instances of a string with treemaps?

Question

So I got a treemap to which I add courses that come from a random generator, the names are consistent but the order is not. For example I got these:

List course = new ArrayList();             
        course.add("COP2210");
        course.add("COP2250");
        course.add("ENC1250");
        course.add("MLP1337");
        course.add("ENC3250");
        course.add("REL2210");
        course.add("MUS3200");
        course.add("PHI1240");
        course.add("SOL5000");
        course.add("HAL9000");

Now that's ten courses in total, and while the code below works to count one of them it would mean i have to get about nine more int variables written and repeat the if statement another nine times, that would be undesirable. I should mention the treemap is inside a loop and resets everytime, which is something I want. but a second treemap can be made outside the loop to add the courses, i got no issue with that. But it is entirely possible that some courses never even gets added to the treemap as they are randomly chosen from the ten above.

for (int i = 0; i < NumberOfStudents; ++i){

        order = RandomCourse();

        TreeMap<String, Integer> tmap = new TreeMap<String, Integer>();
        tmap.put(courses.get(order[0]).toString(), 0);
        tmap.put(courses.get(order[1]).toString(), 0);
        tmap.put(courses.get(order[2]).toString(), 0);


        String comparator1 = courses.get(order[0]).toString();
        String comparator2 = courses.get(order[1]).toString();
        String comparator3 = courses.get(order[2]).toString();

        if(comparator1 == "HAL9000" || comparator2 == "HAL9000" || comparator3 == "HAL9000")
        HAL9000students++;




        courses.indexOf(order[0]);

        //Clean variable
        SortedCourses = "";

        //Map logic
        Set set = tmap.entrySet();
        Iterator iterator = set.iterator();
        while(iterator.hasNext()) {
         Map.Entry mentry = (Map.Entry)iterator.next();
        SortedCourses = SortedCourses + mentry.getKey() + " ";

      }

        students.add(ID_Prefix + i+ ", " + student.get(i) + " " + SortedCourses);
                                                  }

Order is a int array of size 3. Where a random number is stored, it is then used to choose from courses list a position which corresponds to a course.

Answer 1

Whenever you have a situation where you need to do the same sort of operation for a lot of different values, it should give you a hint not to use individual variables, but rather an array or a collection.

You want to get the number of students in each course. So instead of keeping a variable for every course (what happens if there are 15 courses? 100?), you can use a map from the course name to the number of students in that course.

In this case, since you are already dealing with the indexes of the courses rather than their name, you can actually do that with an array.

int[] studentCounts = new int[course.size()];

This will be, of course, declared outside the loop. In this array, studentCounts[0] will be the number of students in the course indexed 0 ("COP2210"). studentCounts[1] will be the number of students in course index 1 ("COP2250") and so on.

So, if your order array is, for example, {3,5,9}, then you'll need to increment studentCounts[3], studentCount[5] and studentCount[9]:

for ( int courseIndex : order ) {
    studentCounts[courseIndex]++;
}

Your second issue is that you want to print the names of the courses selected for each student ordered alphabetically. You use a TreeMap for that, but there is no reason to use a map - you are not actually using the values of the map, just the keys. So a TreeSet is going to make more sense.

So if you declare

TreeSet<String> sortedCourseNames = new TreeSet<>();

You can add to it inside the loop above:

for ( int courseIndex : order ) {
    studentCounts[courseIndex]++;
    sortedCourseNames.add( course.get(courseIndex) );
}

A Set has a simpler iterator than a Map. You can get the strings directly:

StringBuilder sb = new StringBuilder();
sb.append(ID_PREFIX)
  .append(i)
  .append(',')
  .append(student.get(i))
  .append(' ');

for ( String courseName : sortedCourseNames ) {
    sb.append(courseName)
      .append(' ');
}

students.add( sb.toString() );

---

So, some notes:

• Java has coding conventions. Type names (class,interface,enum) should start with a capital letter (e.g. BigPicture). Method, variable and field names should start with a lowercase letter (e.g. bigPicture), and constants should be all-caps (e.g. BIG_PICTURE). So a variable name like SortedCourses is bad, and so is HAL9000students and ID_prefix. I assumed that the ID prefix is a constant (declared static final String), so ID_PREFIX is the correct name. But if it's a variable, it should be idPrefix.
• Do not use raw types. Never declare anything to be a List, TreeMap, etc., without using the base type. It has to be List<String>, TreeMap<String,Integer>, Set<Map.Entry<String,Integer>> and so on. Using generics properly is important for type safety, and it saves you casting. So don't ignore those warnings about "raw types" that the compiler gives you!
• In my solution, you don't need to compare strings. But if you need to compare strings, you do it by str1.equals(str2), not str1 == str2.

• Don't use the operator + to concatenate strings in loops. It's good enough for something like a = "foo " + b + "bar" - a single string concatenation. But in a loop:

  String result = "";
  for (i = 1; i < 5; i++) {
      result = result + " something else ";
  }
  

  Too many objects are created and discarded. It's better to use a StringBuilder as I have shown.