Welcome to the Treehouse Community
Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.
Looking to learn something new?
Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.
Start your free trialIvan Kazakov
Courses Plus Student 43,317 PointsWhen implementing custom compareTo(), wouldn't it make sense to check for type equality with 'instanceof' operator?
... And throw an Exception if it's not (probably other type of exception should've been chosen):
if(!(obj instanceof Treet)){
throw new IllegalArgumentException("Incompatible object: " + obj.toString());
}
Treet other = (Treet)obj;
//further code goes here...
4 Answers
jcorum
71,830 PointsAgreed about finally. It's definitely not needed here. Everything that follows it could be pulled out. Thanks, and good to hear your thoughts!
Bottom line for me is to use generics, so I don't have to accept an Object, or use instanceof, or throw an exception, or use try... catch... I don't really like try... catch.... I agree with you there. But I think it's better than throwing exceptions. But with generics it's all pretty moot. Thanks for your feedback.
Ivan Kazakov
Courses Plus Student 43,317 PointsThank you for your reply, it's quite reasonable.
Still, if to explicitly cast to match the class that is implementing 'Comparable' interface before calling compareTo() (directly or indirectly via sorting, as in this case), there appears a sort of redundancy when we again cast Object to an implementing class type inside the custom compareTo() implementation.
As I understood the exceptions, throwing one allows the method not to return what it is supposed to, among other purposes. But, as I am currently in progress of studying Java, I may still not be enough competent on the matter.:)
jcorum
71,830 PointsTo implement Comparable the compareTo() method must return an int. So throwing an exception is not a good option. And you should type the parameter to match the class that is implementing the interface. This is unlike equals(), where standard practice is to have the parameter be an Object, and use instanceof the check its type.
jcorum
71,830 PointsActually, my answer wasn't quite complete. If you start this way:
public class Book implements Comparable { . . . }
then you must do this: compareTo(Object o) {. . .} or you get: Book is not abstract and does not override abstract method compareTo(Object) in Comparable. However, if you start this way:
public class Book implements Comparable<Book> { . . . }
then you must do this: compareTo(Book b) {. . .} or you get: Book is not abstract and does not override abstract method compareTo(Book) in Comparable<Book>. I was assuming the latter.
I didn't look at the video because I assumed it would be using generics. But as I see, now that I watch it, that is not so.
So if you accept any Object, then, yes, it makes a lot of sense to test it first, as we are told to do when overriding equals(), using instanceof.
Craig's method will throw an exception if someone asks it to compare Posts, say, with Tweets. It won't be an IllegalArgumentException, but it will be an exception. So if you go without generics, you can throw one explicitly, like you do, or implicitly, like Craig does. If those are the only choices, I go with you and explicit.
I personally don't like code that throws exceptions, however, because it makes for a very unfriendly user experience. So if I had to do it Craig's way (accepting an Object o) I would fix up a try... catch... so I could at least give the client a message, rather than an ugly (and to them meaningless exception). Here's a first try:
public int compareTo(Object o) {
Book b = this;
try {
b = (Book) o;
} catch (ClassCastException e) {
System.out.println("Can't compare anything but books to books");
} finally {
//Book b = (Book) o;
if (equals(b)) {
return 0;
}
if (isbn == b.getIsbn()) {
return 0;
} else {
return isbn.compareTo(b.getIsbn());
}
}
}
Note that Book b has to be declared outside the try... because of scope issues (we need it in the finally...). And the compiler will complain if it is not initialized. So I gave up and just initialized it to the current book, knowing that if the incoming Object is a book it will get re-initialized, and if not, it won't matter.
jcorum
71,830 PointsP.S., I was wrong in saying compareTo() shouldn't throw an exception because it must return an int. What it throws and what it returns are two very different things! :(
Ivan Kazakov
Courses Plus Student 43,317 PointsWell, I think it was probably intentionally simplified in that video. In fact, Craig's code wouldn't ever run into an exception as it performs sorting within a strongly-typed and hard-coded Treets[] array. I hope it would be somehow investigated further with generics.
As for try-catch-finally, I think it should be avoided in method implementations for interface conformity, as it is relatively expensive operation, especially if it eventually ends-up in a loop with large number of iterations.
I'd prefer throwing an exception. In my opinion, it's actually not to give an end-user an unfriendly message, but rather to give a developer an opportunity to encounter it during debugging/testing. And to gracefully handle it at that stage, preferably with some conditional checking rather than with try-cathces.
For 'finally' clause - I think, it shouldn't contain anything other than rock-solid guaranteed error-free statements, as it executes always, regardless of what had happened in try-catch.