• Post Reply Bookmark Topic Watch Topic
  • New Topic

Throwing Exceptions twice?  RSS feed

 
Alex Walton
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,

I am working through abstract classes within java, and have the below abstract classes and sub classes:





So if you take a look at the abstract class Sequence, what I am doing is calling validate initially on every Sequence, to check that it is a valid one.
When I come to DNA Sequence, I am trying to create the method revComp() which reverses the DNA sequence and then finds the complimentary letters. This is all ok, but I am slightly confused about throwing the InvalidSequence in the revComp() method. It obviously throws the InvalidException when, in the main of the DNASequence, I create a new DNA Sequence, as this extends the Abstract class Sequence, and validate() is called. So when throwing the InvalidException in the revComp() it makes no difference what parameters I use. This is of course wrong, so how do I get it to throw the exception correctly?

Any help appreciated!

 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There are much simpler ways to validate the entry, but I would suggest that your validate() method is incorrect. That method is behaving a bit like I might if you really annoyed me. Let's say you had really annoyed me and showed me a sequence of base letters and I went through them and found only ACTG. “Go away and stop bothering me; you can see for yourself it is all right.”. But what if I found a U in it and thought you were trying to palm me off some RNA. Instead of pointing out the mistake I hit you with a copy of Head First Java. Hard.
What you are advertising is a method which tests whether the text is valid. So you shou‍ld calm it down, buy it some beer, ask why it is all annoyed, take the flak and wait until it won't hit you. Then when the method gets to the U it won't do anything nasty; it will simply tell you the text ain't right. Maybe make the method return false rather than throw any exceptions. Now you want this sort of thing in the constructor:-Or give validate() void return type and only throw the exception if you find an error.
Let's have a look at the validate() method.
  • 1: It is called from the constructor, so it shou‍ld have private access or be marked final (but not both).
  • 2: Rather than having the complicated three‑args constructor for your Exception, create a String as a message to pass to the exception's constructor. That way you can simply give your exception class constructors with super(...); in and nothing else (see Throwable).
  • 2½: Consider an error message like this if you are going to throw an Exception: "Incorrect character U found at index 123456 of input."
  • 3: I cannot tell whether your Exception is checked or unchecked, but it is behaving rather like an IllegalArgumentException so it shou‍ld probably be unchecked. In which case leave out the throws clauses and put a description of the Exception in the documentation comments with the @exception tag.
  • 4: I am not happy about your loop. I shall have a look at it and write another post.
  •  
    Campbell Ritchie
    Marshal
    Posts: 56536
    172
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    This is what your code looks like when you remove the unnecessary comments and excess blank lines. Your documentation comments are incomplete, but you can sort that out later.Now, there are all sorts of things. Line 45: Inappropriate response to an exception. The exception shou‍ld carry information about the error, and you shou‍ld be able to get that infomation from the getMessage() method, printStackTrace() etc. Use that information rather than simply END.
    Line 23: Avoid += with Strings. Create a StringBuilder object and use its append() method. Consider a switch rather than multiple ifs. Is there any possibility of ever getting to line 31 to throw that exception at all?
    Having had another look at your loop, I think it will work correctly. I was confused by the assignment to false at its beginning. You can however simplify it a great deal. One way is to use a regular expression but that may give slow performance for long Strings (remembering DNA sequences can have millions of letters in). Let's try something:-In both cases the && in the for loops' headings means the loops will stop whenever a certain value occurs (false in the first, true in the second).
     
    Junilu Lacar
    Sheriff
    Posts: 11477
    180
    Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    You don't have to put a main() method in every class you define. If you want to write some test code, write a separate class just for that and keep any kind of test code out of your Sequence class and its subclasses. The test class is the one that will have a main() method, not your other ones. Alternatively, you can use JUnit to test your Sequence classes.
     
    Junilu Lacar
    Sheriff
    Posts: 11477
    180
    Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Is this an exercise that requires you to throw an exception from your constructor if the sequence is invalid?  If not, there are other design options you might want to consider.  For example, you can choose to make your class more tolerant and just mark invalid parts of the sequence as such without necessarily rejecting the good parts. DNA isn't perfect; it could be damaged, right?
     
    Junilu Lacar
    Sheriff
    Posts: 11477
    180
    Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    It's nice to see that you're trying to document your code. However, the documentation comments you've written don't follow the recommended JavaDoc conventions.  See this article: http://www.javaworld.com/article/2072490/more-effective-javadoc.html?page=2 and follow the links in the Use Standard JavaDoc Conventions section towards the end of the article.

    Some methods don't really need to have JavaDocs. These comments are redundant:

    These kinds of comments are also redundant as long as you format your code properly, which you pretty much have.

    Comments like these just add clutter to your code, IMO. They're window dressings that don't tell the reader anything he can't already figure out just by reading what the code says.
     
    Junilu Lacar
    Sheriff
    Posts: 11477
    180
    Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Here's another section of your code that could use a little bit of cleaning up. You may think we're digressing and nitpicking at things that are unrelated to your main question but these relate to clarity of design. By addressing these issues, you might end up rethinking your entire approach around exceptions and handling invalid input.

    Once again, your method comment doesn't follow standard conventions and it doesn't add much value. Campbell also pointed out that since it is called from the constructor, you should make it either private or final. Let's make it private:

    Making a private method throw an exception like InvalidSequenceException doesn't make a lot of sense. Only the enclosing class can call this method so why bother with throwing an exception from here? Besides, you've declared the method to return a boolean value so why not return false instead of throwing an exception when you find invalid content? So now, the method becomes just:

    Try fixing up validate() so that it doesn't throw any exceptions but instead will always return true or false. Then see if you can improve your JavaDoc comment for the method or see if you need any comment there at all.
     
    Campbell Ritchie
    Marshal
    Posts: 56536
    172
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Junilu Lacar wrote:. . . Making a private method throw an exception like InvalidSequenceException doesn't make a lot of sense. . . .
    The problem is that OP has managed to write a method which doesn't only validate the DNA, but records an error message about it. You now have a situation where you are trying to return two things and you can't. One way to do it is to throw an Exception which encapsulates the error, with a message like:-
    Incorrect base = U found at index 123456: not checking any further.
    Now I have seen what you wrote, agreeing that such a method shou‍ld return false/true, I have thought about how one would try to return two things from a method. And isn't there a standard answer?That will need quite a bit of refinement before you can release it on an unsuspecting world. Particularly lines 37‑38, which offer lots of scope for refinement. Also some spellling corrrections.
     
    Junilu Lacar
    Sheriff
    Posts: 11477
    180
    Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    This is why I like Test-Driven Development. The problem with the OP's approach in writing this code is that it focuses internally, on what's happening inside the class. Let's switch perspective and focus externally, to what client code that uses these Sequence classes might look like and behave.

    Before you go on trying to "fix" the Sequence class code, think about what makes sense from the client point of view when it tries to instantiate these classes. Would it have to handle the exception? What would the client code look like then? Is that what you want to do every time you create one of these objects? Does it make sense? If not, what does?
     
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!