Java - Issue with Collections.sort()

Question

I am currently writing a program that prints an ArrayList of books. Each ArrayList of books elements consist of a string (the title of a book) and an ArrayList (the authors of that book). I need to sort my ArrayList of books so that they appear in alphabetical order (sorted by titles). My issue is that when I print the new ArrayList (the list that I call Collections.sort() on) I get the same output as the first time I printed the non-sorted version.

I am calling myLib.sort(); from my driver program which goes to this method in my Library class:

   public void sort()
   {
      Collections.sort(myBooks);
   }

myBooks is the ArrayList of books I mentioned earlier. From what I've read, Collections.sort("ArrayList name") should sort my list alphabetically. If that is incorrect and I need to use compareTo() and equals() methods, then here are those methods as they appear in the class Book that I use to construct the books that go into my class Library:

   public int compareTo(final Book theOther)
   {
      int result = 0;
      if (myTitle.equals(theOther.myTitle))
      {
         if (myAuthors.get(0) != theOther.myAuthors.get(0))
         {
            result = 1;
         }
      }
      else
      {
         result = 0;
      }
      return result;
   }
   public boolean equals(final Object theOther)
   {

      if (theOther instanceof String)
      {
         String other = (String) theOther;
         return myTitle == other;
      }
      else
      {
         return false;
      }  
   }

The only remaining possible issue that I can think of is with my printing method. My driver program prints myLib which is a Library. My Library class has the following toString() method:

   public String toString()
   {
      String result = "";
      for (int i = 0; i < myBooks.size(); i++)
      {

         String tempTitle = myBooks.get(i).getTitle();
         ArrayList<String> tempAuthors = myBooks.get(i).getAuthors();
         Book tempBook = new Book(tempTitle, tempAuthors);
         result += (tempBook + "\n");
      }
      return result;
   }

This gets each book and that book's string from my Book class toString() method which is the following:

   public String toString()
   {
      return "\"" + myTitle + ",\" by " + myAuthors;
   }

If this was too little, too much, too confusing, not clear enough, etc... Please let me know in a comment and I will edit the post ASAP. I can also post the entirety of my three classes if need be. I am new to Java and fairly new at posting so I'm still getting used to how things are done in both cases so I'd appreciate it if you'd go easy on me. Thank you!

@BachT When I do that my program prints things like Book@5c647e05 for each of my books. — Trafton
– Trafton, Commented Aug 5, 2015 at 2:07
public int compareTo(final Book theOther) { int result = myTitle.compareTo(theOther.myTitle); if (result == 0) { if (myAuthors.get(0) != theOther.myAuthors.get(0)) { result = 0; } } return result; } — BachT
– BachT, Commented Aug 5, 2015 at 2:14
@BachT Should I edit my post and add all code, or would it be better if I did that in a private chat with you? — Trafton
– Trafton, Commented Aug 5, 2015 at 2:21

Flowryn · Accepted Answer · 2015-08-05 02:52:40Z

Your compareTo() method seems to be wrong, note that Collections.sort() uses that method to compare the objects in your list.

You only check if the titles are equals, if they are then you compare the first authors and if they are equal you return 1, else you return 0;

compareTo() is used for check if this object is less, equals or greater than the one you are comparing with(returning 0 is for equals, a negative number for less and positive for greater, you return either a positive number either 0). I recommend you to read the javadoc for compareTo() method.

As an example here is an implementation of Book class where I only compare according to title (I omitted the comparison for the author list).

public class Book implements Comparable<Book> {
    private String title;
    private List<String> authors;

    public Book(String title) {
        this.title = title;
    }

    public int compareTo(Book o) {
        return this.title.compareTo(o.title);
    }

    @Override
    public boolean equals(Object b){
        if(!(b instanceof Book)){
            return false;
        }
        //authors comparison omitted
        return this.title.equals(((Book) b).title);
    }

    @Override
    public String toString(){
        return "Title: "+ title; //todo: add authors also if need
    }     
}

As you see in Book.compareTo() method I rely on the String.compareTo(). it will return -1, 0 or 1; if you need to compare according to author list also you have to thing how will be the logic of the method and think of some issues:

if is enough to rely only on the first authors on the list
if you need to make sure that list of authors is sorted
what happens if the author list is empty

Also NOTE: compareTo should be consistent with equals which means if compareTo returns 0 then equals should return true and vice versa.

Community · Accepted Answer · 2017-05-23 12:14:00Z

2

According to the documentation, you should also return negative value:

Returns a negative integer, zero, or a positive integer as this object is less than, equal to, or greater than the specified object.

public int compareTo(final Book theOther) {
    int result = myTitle.compareTo(theOther.myTitle);
    if (result == 0) {
        result = myAuthors.get(0).compareTo(theOther.myAuthors.get(0));
    }
    return result;
}

check @flowryn for better answer, as he also mention about equals() according to the documentation:

It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

edited May 23, 2017 at 12:14

CommunityBot

11 silver badge

answered Aug 5, 2015 at 2:44

BachT

1,0689 silver badges17 bronze badges

6 Comments

Flowryn Over a year ago

Is incomplete, note that @Trafton does not ensure the consistency between equal and compareTo which may lead to some unexpected behavior

BachT Over a year ago

Yes, you are correct about the consistency. But I think, only for the problem of sorting, as the case of OP, we only need to implement Comparable, so the only method need to be implemented was compareTo(). equals() will be used for other operations.

Flowryn Over a year ago

See the documentation that you linked to: It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y))

BachT Over a year ago

Yep, I agree with you: we should have equals() or others will screw up. But (i think) for this case, it is not strictly required.

Flowryn Over a year ago

if you will not interfere with other collections then is ok.

|

Collectives™ on Stack Overflow

Java - Issue with Collections.sort()

2 Answers 2

Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related