2

I don't think that's the best way to word that title but I can't think of a better way to word it. Here's my problem: I have to write a method that compares in several different ways. If the last names are the same, I then need to compare by first name. If the first names are the same, then I need to sort by section. What would be the most effective way to sort a data structure in this hierarchy? Here's what I've currently got and I think I understand why it doesn't work but I can't come up with a different way to write this function:

//Student class structure, each field has a public get/set method
public class Student implements Comparable<Student>
{
  private String fname;
  private String lname;
  private int section;
}

//My current compareTo method
@Override
public int compareTo(Student s)
{
  /*
    -compare by last name
    -if the same, compare by first name
    -if the same, compare by section
  */

  String slast = s.getLastName();

  if(lname.compareTo(slast) == 0)
  {
    String sfirst = s.getFirstName();

    if(fname.compareTo(sfirst) == 0)
    {
      int sclass = s.getSection();

      return Integer.compare(section, sclass);
    }

    else
    {
      return fname.compareTo(sfirst);
    }
  }

  else
  {
    return lname.compareTo(slast);
  }
}
  • Does it work and you are looking for a better way to do this or do you have any issues with this code? If it is the latter, please add more details on what input it fails to give the expected result – Thiyagu Jan 21 '20 at 15:46
  • 1
    Does this answer your question? [How to compare objects by multiple fields](https://stackoverflow.com/questions/369512/how-to-compare-objects-by-multiple-fields) – Scratte Jan 21 '20 at 15:52
  • @user7 my mistake, my problem with my code was in a problem that i didn't expect it to be in, deleting the question, my bad. – John Porterfield Jan 21 '20 at 15:53
  • @Scratte that does help, thank you – John Porterfield Jan 21 '20 at 15:53

3 Answers3

4

You can create a Comparator for your Student class this way:

Comparator<Student> comparator = Comparator
        .comparing(Student::getLastName)
        .thenComparing(Student::getFirstName)
        .thenComparing(Student::getSection);

And then use this comparator (instead of implementing Comparable interface) to sort a list with Student objects, or to create a TreeMap with these objects:

Collections.sort(listOfStudents, comparator);
TreeMap<Student> mapOfStudents = new TreeMap<>(comparator);
ardenit
  • 3,610
  • 8
  • 16
1

You don't have to use getters or setters if you're overriding compareTo. You can also forgo the else/return statements since they're terminal return statements, and just use return.

@Override
public int compareTo(Student s) {
    if (lname.compareTo(s.lname) == 0) {
        if (fname.compareTo(s.fname) == 0) {
            return section.compareTo(s.section);
        }
        return fname.compareTo(s.fname);
    }
    return lname.compareTo(s.lname);
}
Compass
  • 5,867
  • 4
  • 30
  • 42
1

Your code looks correct to me.

What would be the most effective way to sort a data structure in this hierarchy?

Well, it's worth mentioning that you are potentially doing the first two comparisons (first name and last name) multiple times

if(lname.compareTo(slast) == 0)
{
    //...
}
else
{
    return lname.compareTo(slast);
}

It should be fairly obvious that you are doing lname.compareTo(slast) twice. You can store the result in a variable instead.

int lastNameComparison = lname.compareTo(slast);
if(lastNameComparison == 0)
{
    //...
}
else
{
    return lastNameComparison;
}

It is a matter of style, but I would not bother to store the result of getters into variables. Just call them when you need them.

Combining both of the above points, you get:

int lastNameComparison = lname.compareTo(s.getLastName();
if (lastNameComparison == 0)
{
    int firstNameComparison = fname.compareTo(s.getFirstName());
    if (firstNameComparison == 0)
    {
        return Integer.compare(section, s.getSection());
    }
    else
    {
        return firstNameComparison;
    }
}
else
{
    return lastNameComparison;
}

The nesting is quite ugly and if we need to add another criteria, it would get even worse.

We can solve that by inverting the conditions and using multiple return statements.

int lastNameComparison = lname.compareTo(s.getLastName());
if (lastNameComparison != 0) return lastNameComparison;

// Last names must be equal
int firstNameComparison = fname.compareTo(s.getFirstName());
if (firstNameComparison != 0) return firstNameComparison;

// First names must be equal
return Integer.compare(section, s.getSection());

I would personally use the declarative style of writing this, but if this code is for an assignment, that is likely not what they are expecting.

Michael
  • 41,989
  • 11
  • 82
  • 128