Constructor throwing runtime exception

Question

I have a constructor that takes in a string as a parameter. I want to throw a runtime exception everytime the string that is passed into the constructor contains anything that is not either "A", "C", "G", or "T". Currently this is what my code looks like:

public DNAStrandNovice(String strand) {
    passedStrand = strand;
    if (passedStrand.contains("a") || passedStrand.contains("c")
            || passedStrand.contains("g") || passedStrand.contains("t")) {
        throw new RuntimeException("Illegal DNA strand");
    } else if (passedStrand.contains("1") || passedStrand.contains("2")
            || passedStrand.contains("3") || passedStrand.contains("4")
            || passedStrand.contains("5") || passedStrand.contains("6")
            || passedStrand.contains("7") || passedStrand.contains("8")
            || passedStrand.contains("9") || passedStrand.contains("0")) {
        throw new RuntimeException("Illegal DNA Strand");
    } else if (passedStrand.contains(",") || passedStrand.contains(".")
            || passedStrand.contains("?") || passedStrand.contains("/")
            || passedStrand.contains("<") || passedStrand.contains(">")) {
        throw new RuntimeException("Illegal DNA Strand");


    }
    }

I feel like this could be implemented in a much more concise way, but I don't know how. Right now I'm just checking for every character that is not the capital letters "A", "C", "G", or "T" and throwing a run time exception but I feel like it's too tedious and bad programming style. Anyone have any ideas?

Not a good idea to throw an exception from a constructor. see http://stackoverflow.com/questions/1371369/can-constructors-throw-exceptions-in-java — Scary Wombat, Jan 31 '14 at 02:10
Why are throwing exception when the string *`contains`* what you want? Shouldn't you throw exception only when it *doesn't* contain what you want? — ADTC, Jan 31 '14 at 02:18
It's throwing an exception when the string that is passed into the constructor contains an illegal character for this assignment such as "?" or anything that is not "A", "C", "G", or "T". — user122947, Jan 31 '14 at 02:20
Then you should check for the valid values and throw exception on any string that does not match valid data. You should also only accept strings that are **one** character long. *Please see my answer below.* — ADTC, Jan 31 '14 at 02:27

score 2 · Answer 1 · answered Jan 31 '14 at 02:15

2

Check negatively, instead of positively.

for (int i = 0; i < str.length(); i++) {
   if (str.charAt(i) != 'A' && str.charAt(i) != 'C'
       && str.charAt(i) != 'G' && str.charAt(i) != 'T') {
     throw new IllegalArgumentException("Bad character " + str.charAt(i));
   }
}

...or, even shorter,

for (int i = 0; i < str.length(); i++) {
  if (!"ACGT".contains(str.charAt(i))) {
    throw new IllegalArgumentException("Bad character " + str.charAt(i));
  }
}

answered Jan 31 '14 at 02:15

Louis Wasserman

191,574
25
345
413

The first for loop wouldn't work if the strand consisted of just "A" "G" and "C" letters. It should rather throw an exception when it encounters anything other than a "A", "C", "G", or "T". I do understand what you mean by checking negatively instead of positively though. – user122947 Jan 31 '14 at 02:19
@user122947: I don't follow. So what if the strand contains just "A" "G" and "C" letters? Your problem statement seems to imply that that's an allowed input. (Assuming this is meant to resemble a problem in genetics, that too would be a valid interpretation.) – Louis Wasserman Jan 31 '14 at 02:20
Ahhhhhh, I'm sorry Louis. I just read through and checked the code and it does indeed fix my problem. I see it now. Sorry for confusing you there. Thank you so much for your help. – user122947 Jan 31 '14 at 02:28
@user122947 Regex offers a simpler but more powerful alternative to using `for` loops to check. You can do a lot more than simply checking for valid characters. `=)` – ADTC Jan 31 '14 at 02:54
That's true, but then you're incurring all the performance overhead of the regex engine to do something very simple. – Louis Wasserman Jan 31 '14 at 02:55
I highly doubt there's that much of a performance overhead to consider, or even if performance is *that much* of a concern to OP. Considering what the application looks like, the OP may want to make the checks more complex in the future. Regex makes it very easy to build more complex checks, while with the *hard-check* code, you will have to make the code more complex and convoluted. – ADTC Jan 31 '14 at 03:00

ADTC · Answer 2 · 2014-01-31T02:52:55.010

You can achieve this using regex (regular expressions):

public DNAStrandNovice(String strand) {
    if (!strand.matches("[ACGT]+")) { //or [ACGT]   <-- see note below
        throw new RuntimeException("Illegal DNA strand");
    }
    passedStrand = strand;
}

The regular expression [ACGT]+ means the string must have one or more characters, and each of them must be one of A, C, G or T. The ! in front of strand.matches reverses the boolean value returned by matches, essentially meaning if the string does not match the regex, then throw RuntimeException.

Note: If you need the string to have exactly one character, use the regex [ACGT]. If you need to allow spaces, you can use [ACGT ]+ (then trim and check for empty) or [ACGT][ACGT ]+ (which ensures the first character is not a space).

You can even do much more complex and powerful regex checks such as patterns that should contain exactly four characters repeated with spaces in between (example ATCG TACG) or even where only certain characters appear in certain places, like only A and C can appear as first two characters, and only G and T can appear following it (example ACTG is correct while AGTC is wrong). I will leave all that as an exercise.

score 1 · Answer 3 · answered Jan 31 '14 at 02:25

Recommend against using an exception. Define an Enum and pass that.

public enum DnaCode { A, C, G, T }  
...  
public DNAStrandNovice(List<DnaCode> strand) {  
...
}

Or make it a DnaCode[] if you prefer. You can control the input and avoid dealing with interrupted control flow. Exceptions are rather expensive to throw and are not really intended for use as a method of flow control.

score 0 · Answer 4 · answered Jan 31 '14 at 02:13

0

You can make the code slightly more efficient by manaully looping through the characters and checking for the letters either with ifs or a Set.

But honestly, unless performance is a problem, it's good how it. Very obvious and easy to maintain.

answered Jan 31 '14 at 02:13

Ted Bigham

4,237
1
26
31

I honestly feel like this is a very inefficient way to do it. It would literally take over 60 lines of code to check for every character. How would I set up the loop so that it throws an error everytime it encounters a character that is not either "A", "C", "G", or "T"? – user122947 Jan 31 '14 at 02:17

score 0 · Answer 5 · answered Jan 31 '14 at 02:18

I was going to jump in with a possibility...

public boolean validateLetter(String letter){
    HashMap<String, String> dna = new HashMap<String, String>();
    dna.put("A", "A");
    dna.put("C", "C");
    dna.put("G", "G");
    dna.put("T", "T");


    if(dna.get(letter) == null){
        System.out.println("fail");
        return false;
    } else {
        return true;
    }



}

I would also not put that code in the constructor, rather put it in its own method and call from the constructor.

score 0 · Answer 6 · answered Jan 31 '14 at 02:27

public DNAStrandNovice(String strand){

     if(strand.matches("^[A-Za-z]*[0-9]+[A-Za-z]*$") || strand.matches("^[a-zA-Z]*[^a-zA-Z0-9][a-zA-Z]*$") || strand.matches("^[A-Za-z]*[acgt]+[A-Za-z]*$")){

                  throw new RuntimeException("Illegal DNA strand");
      }

}

Constructor throwing runtime exception

6 Answers6