CODEWITHSUNDEEP
java
user122947
user122947

Reputation: 1

Constructor throwing runtime exception

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?

Upvotes: 0

Views: 700

Answers (6)

ADTC
ADTC

Reputation: 10086

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.

Upvotes: 2

phntmasasin
phntmasasin

Reputation: 775

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");
      }

}

Upvotes: 0

unigeek
unigeek

Reputation: 2826

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.

Upvotes: 1

badperson
badperson

Reputation: 1614

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.

Upvotes: 0

Louis Wasserman
Louis Wasserman

Reputation: 198103

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));
  }
}

Upvotes: 2

Ted Bigham
Ted Bigham

Reputation: 4341

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.

Upvotes: 0

Related Questions