Forum:

Beginning Java

Beginner Java code review: RNA Transcription exercise

Greenhorn

Posts: 8

posted 5 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Hi everyone,
I’m a beginner learning Java and I recently finished a small learning project based on Exercism’s RNA Transcription exercise.

The program converts a DNA string into its RNA complement using standard transcription rules.

I’d really appreciate feedback on:

- Code readability and naming

- Class and method design

- Whether my solution follows Java best practices

- What you would improve or do differently as an experienced developer

GitHub repository: https://github.com/asantana4/java-rna-transcription

Here is the code:

public record DNAStrand(String dnaNucleotidesSequence) {

public DNAStrand(String dnaNucleotidesSequence) {
        this.dnaNucleotidesSequence = dnaNucleotidesSequence.toUpperCase();
    }

record RNAStrand(String rnaNucleotidesSequence) {

@Override
        public String toString() {
            return rnaNucleotidesSequence;
        }
    }

private String transcribeRNA() {
        StringBuilder rnaNucleotideSequence = new StringBuilder();
        for (int i = 0; i < dnaNucleotidesSequence.length(); i++) {
            char nucleotide;
            switch(dnaNucleotidesSequence.toUpperCase().charAt(i)) {
                case 'C' -> nucleotide = 'G';
                case 'T' -> nucleotide = 'A';
                case 'A' -> nucleotide = 'U';
                default -> nucleotide = 'C';
            }
            rnaNucleotideSequence.append(nucleotide);
        }
        return rnaNucleotideSequence.toString();
    }

public RNAStrand getRNACompliment() {
        return new RNAStrand(transcribeRNA());
    }

@Override
    public String toString() {
        return dnaNucleotidesSequence;
    }
}

Thanks in advance for your time and feedback.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 5 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Welcome to the Ranch

Please post the code here and let's have a look at it.

Java 8 (verified skill)
Skill verified by Jeanne Boyarsky

Tim Holloway

Bartender

Posts: 29139

215

I like...

posted 5 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Welcome!

There's a button on our message editor labelled "Code". It will insert Code tags to wrap around your sample code so you can copy-paste it and not lose formatting. That will make it easier to read.

Please DON'T post screen shots, though! Just paste text. It's easier on our eyes and on our database storage!

The problem with getting rid of the "undesirables" is that sooner or later someone will decide that YOU are an undesirable.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 5 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Thank you for showing the code.

That looks like proper object‑oriented code; whoever is teaching you is doing a good job.

I am pleased to see you using a record, which I think is a good option, but please explain why you chose a record.

Why have you got the second record as an inner class? That looks strange. It it were an inner class, I think it is best to make it private.

Your method to get the complementary strand (not compl[u]iI/u]ment) creates a new instance every time it is called.

Please suggest some other ways to calculate the coplement other than using a switch‑case. You might consider a Map, if you are familiar with Maps.

I don't like to see default for a normal input after case. Restrict default to abnormal cases, for example so you can throw an exception. Have cases for A, c, T, and G. Use default to signal an abnormal letter and throw the appropriate exception.

Run the complement method once and once only, from the constructor. Use it to initlialise a field. Remember Strings are immutable.

I hope that helps; there would be more comments if I had more time.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 5 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Why are you returning different types from the toString() and getXXX() methods?

Give the transcription method void instead of its return type and use it to create the complementary strand field. If you give that method private access, and only call it from the constructor, then it will only be called once.

Consider returning something representing both strands from toString().

Carey Brown

Bartender

Posts: 11226

I like...

posted 5 months ago

2
Number of slices to send:

Optional 'thank-you' note:

Send

JavaRanch-FAQ HowToAskQuestionsOnJavaRanch UseCodeTags DontWriteLongLines ItDoesntWorkIsUseLess FormatCode JavaIndenter SSCCE API-17 JLS JavaLanguageSpecification MainIsAPain KeyboardUtility

Campbell Ritchie

Marshal

Posts: 82459

594

posted 5 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

That looks a nice solution, Carey. Please explain more about it. For example, you seem to be using a functional style there Also please explain the use of a Stream, which a beginner might not be familiar with.
Why did you miss out G? Should you have given that class a private constructor?

Mike Simmons

Rancher

Posts: 5304

posted 5 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Or, adapting Carey's solution to use something like that nice switch expression Alexander already came up with, and also use Collectors.joining():

Alexander Santana

Greenhorn

Posts: 8

posted 4 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Campbell Ritchie wrote:Thank you for showing the code.

That looks like proper object‑oriented code; whoever is teaching you is doing a good job.
I am pleased to see you using a record, which I think is a good option, but please explain why you chose a record.
Why have you got the second record as an inner class? That looks strange. It it were an inner class, I think it is best to make it private.
Your method to get the complementary strand (not compl[u]iI/u]ment) creates a new instance every time it is called.
Please suggest some other ways to calculate the coplement other than using a switch‑case. You might consider a Map, if you are familiar with Maps.
I don't like to see default for a normal input after case. Restrict default to abnormal cases, for example so you can throw an exception. Have cases for A, c, T, and G. Use default to signal an abnormal letter and throw the appropriate exception.
Run the complement method once and once only, from the constructor. Use it to initlialise a field. Remember Strings are immutable.
I hope that helps; there would be more comments if I had more time.

Thank you very much for the feedback Campbell Ritchie!

Sorry for the delayed reply. I want to address your points in the order you raised them.

1- I chose to use a record because I had just learned about them and wanted to practice using them to get familiar with it. Do you think a record is a good choice for a program like this?
2- The reason I have the other RNAStrand record as an inner record is because I didn't want the outside code to have direct access to it, which means you are absolutely right about making the inner record private. When you say it looks strange, do you mean having a record inside another record in general or is it something specific about the way I did it?
3- Thanks for pointing out that the getRNAComplement method returns a different instance everytime it's called. I totally missed this, because the purpose is to have one RNA complement per DNAStrand.
4- I thought about your question of using a different way to calculate the component other than a switch-case. I could only think of String.replace. I am not familiar with Maps, but I will look into it and try that approach. Thank you so much for suggesting it!
5- You are right about the default case in switch. I honestly didn't think about the purpose of the default case.
6- I will definitely run the complement method only once. Now, when you mentioned Strings are immutable, do you mean I have to avoid modifying Strings since new objects are created when I do?

Thanks again for taking the time to review my code. This has has been extemely helpful. I will refactor the code and post an updated version.

Alexander Santana

Greenhorn

Posts: 8

posted 4 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Campbell Ritchie wrote:
Why are you returning different types from the toString() and getXXX() methods?
Give the transcription method void instead of its return type and use it to create the complementary strand field. If you give that method private access, and only call it from the constructor, then it will only be called once.
Consider returning something representing both strands from toString().

I see your point Campbell Ritche. Thank you very much!

Campbell Ritchie

Marshal

Posts: 82459

594

posted 4 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Alexander Santana wrote:. . . Thank you very much . . .

That's a pleasure

It is good to see you back. And it is good to see such clear explanations for design decisions.

. . . I chose to use a record because I had just learned about them . . .

That is a good reason

Do you think a record is a good choice . . .?

Yes. A record is (or should be) immutable, which has all sorts of advantages. Look in books like Effective Java by Joshua Bloch, and look for the section called “Minimize Mutability” or similar.

2- The reason I have the other RNAStrand record as an inner record is because I didn't want the outside code to have direct access to it . . .

You can probably do that more simply with a private field. What you are describing is the standard practice of information hiding.

. . . Thank you so much for suggesting it!

The idea of coming to a forum is to let many people look at your code and make suggestions. If you produce more code, you will probably get more suggestions and help.

. . . Strings are immutable . . . I have to avoid modifying Strings . . .

It means you cannot modify a String object. If you go through the String documentation and look for methods, you won't find any that appear to modify the object and have void instead of a return type. They all return String, which means a new object. Any modification is not your problem. If you have a String field, you can return a reference to it 1,000,000× and your users can play with it to their hearts' content. Since they always create new objects, your original cannot change, and your record is safe from such changes. That is why the documentation says Strings can be shared.
Since records are supposed to be immutable, I think it is a good idea to initialise the RNA immediately, before the constructor completes, otherwise calculating the RNA will alter the state of the record object and it will become mutable. You can work a fiddle by ensuring that the RNA is only calculated once and the change can never be visible to users; you can call that functional immutability. But initialising all the fields from the cnstructor is probably an easier way to ensure immutability.
Look forward to seeing your updated version.

Alexander Santana

Greenhorn

Posts: 8

posted 4 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Carey Brown wrote:import java.util.Map; public class TransscribeString { private static final Map<Character,Character> xform = Map.of( 'C', 'G', 'T', 'A', 'A', 'U' // default 'C' ); public static String transcribe( String s ) { return s.toUpperCase().chars() .mapToObj(code -> (char) code) .map(ch -> xform.getOrDefault(ch, 'C')) .collect(StringBuilder::new, StringBuilder::append, StringBuilder::append) .toString(); } }

Carey Brown, thank you for sharing an alternative implementation. It is much appreaciated.

Alexander Santana

Greenhorn

Posts: 8

posted 4 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Mike Simmons wrote:Or, adapting Carey's solution to use something like that nice switch expression Alexander already came up with, and also use Collectors.joining():
public static String transcribe( String s ) { return s.chars() .mapToObj(TranscribeString::transcribeCode) .collect(Collectors.joining()); }

Thanks a lot for sharing your implementation Mike Simmons!

Alexander Santana

Greenhorn

Posts: 8

posted 4 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Campbell Ritchie wrote:. . .
Since records are supposed to be immutable, I think it is a good idea to initialise the RNA immediately, before the constructor completes, otherwise calculating the RNA will alter the state of the record object and it will become mutable. You can work a fiddle by ensuring that the RNA is only calculated once and the change can never be visible to users; you can call that functional immutability. But initialising all the fields from the cnstructor is probably an easier way to ensure immutability.
Look forward to seeing your updated version.

I am thinking about making DNAStrand a class with two private fields: String dnaNucleotideSequence and RNAComplement rnaComplement where RNAComplement is a private inner record.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 4 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Have corrected your quote tags.
Let's throw a few random ideas into the air and see whether you like them.
It might be better to have the source strand and the destination strand fields of the same type, or nearly the same type. Maybe an unmodifiable List<Nucleotide> and you make Nucleotide into an enum. Or two enums, one with T and one with U. You can use enums as cases in switch statements or as keys in a kind of Map. This may all be beyond your current knowledge and experience.
I know it is easy to use a String as input and result, but beware of Strings. One problem you might have is that you cannot be sure that a String only contains ACTG/ACUG. I shall let you think how you can get round that with the default keyword in a switch statement. Strings do have the great advantage, as I said last night, of being immutable.
It would be easy to use Nucleotide[]s as the fields, but arrays are mutable. See Joshua Bloch's book about defensive copies. Bloch recommends the clone() method for copying arrays. You would have to copy an array if it comes in via the constructor and also whenever one goes out via a method, otherwise distant code can change the state of your object. So you end up creating many new objects.
As I said, random thoughts; let's see whether you can make any use of them.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 4 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Yesterday, I wrote:. . . . Effective Java by Joshua Bloch, and look for the section called “Minimize Mutability” or similar.

2nd edition page 73 or 3rd edition page 80.

. . . Joshua Bloch's book about defensive copies. . . .

2nd edition page 184 or 3rd edition page 231.

Harini Alagarsamy

Greenhorn

Posts: 6

posted 4 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

This discussion was very helpful for me as a Java beginner.
I learned about immutability, use of records, and alternative approaches
like Maps and switch expressions. Thank you for the clear explanations.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 4 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Welcome to the Ranch

both of you.

Alexander Santana

Greenhorn

Posts: 8

posted 3 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Campbell Ritchie wrote:Have corrected your quote tags.
Let's throw a few random ideas into the air and see whether you like them.
It might be better to have the source strand and the destination strand fields of the same type, or nearly the same type. Maybe an unmodifiable List<Nucleotide> and you make Nucleotide into an enum. Or two enums, one with T and one with U. You can use enums as cases in switch statements or as keys in a kind of Map. This may all be beyond your current knowledge and experience.
I know it is easy to use a String as input and result, but beware of Strings. One problem you might have is that you cannot be sure that a String only contains ACTG/ACUG. I shall let you think how you can get round that with the default keyword in a switch statement. Strings do have the great advantage, as I said last night, of being immutable.

Hello Campbell! Thank you very much for all your ideas and suggestions, I have used some of them. I read the post about Strings being bad and decided to start the refactoring of the program there, the abstraction of the problem. I decided to ask for any feedback in my first post to find areas of improvement in order to tackle (practice) those areas of improvement one by one. So, I just want to focus on practicing one new thing at a time. Right now I want to get better at abstraction (including using Strings the right way). In the case of the refactored rna-transcription program, what do you think about my abstraction of the problem?

Here's the new improved program:

public class Cell {

private final String cellName;
    private final DNAStrand dnaStrand;
    private final RNAm rnaM;

public Cell(String cellName, List<DNANucleotide> dnaNucleotideSequence) {
        this(cellName, dnaNucleotideSequence, true);
    }

public Cell(String cellName, List<DNANucleotide> dnaNucleotideSequence, boolean hasAdnaTemplateStrand) {
        this.cellName = cellName;
        dnaStrand = new DNAStrand(dnaNucleotideSequence, hasAdnaTemplateStrand);
        rnaM = new RNAm(transcribeRNA(dnaNucleotideSequence));
    }

private List<RNANucleotide> transcribeRNA(List<DNANucleotide> dnaNucleotideSequence) {
        RNANucleotide rnaNucleotide;
        RNANucleotide[] rnaNucleotideArray = new RNANucleotide[dnaNucleotideSequence.size()];
        int index = 0;
        for (DNANucleotide dnaNucleotide: dnaNucleotideSequence) {

switch(dnaNucleotide) {
                case CYTOSINE -> rnaNucleotide = dnaStrand.isATemplateStrand? RNANucleotide.GUANINE :
                                    RNANucleotide.CYTOSINE;
                case THYMINE -> rnaNucleotide = dnaStrand.isATemplateStrand? RNANucleotide.ADENINE:
                        RNANucleotide.URACIL;
                case ADENINE -> rnaNucleotide = dnaStrand.isATemplateStrand? RNANucleotide.URACIL :
                        RNANucleotide.ADENINE;
                case GUANINE -> rnaNucleotide = dnaStrand.isATemplateStrand? RNANucleotide.CYTOSINE :
                        RNANucleotide.GUANINE;
                default -> throw new IllegalArgumentException(
                        "Invalid DNA nucleotide: " + dnaNucleotide.name());

}
            rnaNucleotideArray[index] = rnaNucleotide;
            index++;
        }
        return List.of(rnaNucleotideArray);
    }

static class  DNAStrand {
            private final List<DNANucleotide> dnaNucleotideSequence;
            private final boolean isATemplateStrand;

private DNAStrand(List<DNANucleotide> dnaNucleotideSequence, boolean isATemplateStrand) {
                this.dnaNucleotideSequence = dnaNucleotideSequence;
                this.isATemplateStrand = isATemplateStrand;
            }

@Override
            public String toString() {
                StringBuilder nucleotideInitialsSequence = new StringBuilder();
                for (DNANucleotide nucleotide : dnaNucleotideSequence) {
                    nucleotideInitialsSequence.append(nucleotide.name().charAt(0));
                }
                return nucleotideInitialsSequence.toString();
            }
        }

static class RNAm {

private final List<RNANucleotide> rnaNucleotideSequence;
            private RNAm(List<RNANucleotide> rnaNucleotideSequence) {
                this.rnaNucleotideSequence = rnaNucleotideSequence;
            }

@Override
            public String toString() {
                StringBuilder nucleotideInitialsSequence = new StringBuilder();
                for (RNANucleotide nucleotide : rnaNucleotideSequence) {
                    nucleotideInitialsSequence.append(nucleotide.name().charAt(0));
                }
                return nucleotideInitialsSequence.toString();
            }
        }

@Override
    public String toString() {
        return "%s%n%s".formatted(dnaStrand, rnaM);
    }
}

Campbell Ritchie

Marshal

Posts: 82459

594

posted 3 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

I like the enum, but that makes the switch unnecessary. You can record the RNA complement in the enum, which is a much more object‑oriented solution. Look in the Java® Language Specification (=JLS) because there are all sorts of useful examples. You will probably end up with something like this:-Note I have changed the spellings slightly. You can consider doing the same for RnaNucleotide. Now all you need to do to get a sequence is call the appropriate method in order and Bob's your uncle.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 3 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

A few minutes ago, I wrote:. . . Note I have changed the spellings . . .

Also that I inadvertently introduced an error. I shall let you find it

I have always been used to bases in the order ACTG/ACUG.
[edit]Also I fogrot to assign the short title in the constructor.[/edit]

Campbell Ritchie

Marshal

Posts: 82459

594

posted 3 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Not sure, but maybe your nested class should have private access.

Alexander Santana

Greenhorn

Posts: 8

posted 2 months ago

Number of slices to send:

Optional 'thank-you' note:

Send

Campbell Ritchie wrote:

A few minutes ago, I wrote:. . . Note I have changed the spellings . . .

Also that I inadvertently introduced an error. I shall let you find it I have always been used to bases in the order ACTG/ACUG.
[edit]Also I fogrot to assign the short title in the constructor.[/edit]

Thanks for your feedback Campbell Ritchie! Your sample DnaNucleotide enum blew my mind! I also read the Java® Language Specification (=JLS) about the enum class and I did some refactoring. When I was refactoring the nucleotide enums to add the nucleotide complement fields and constructors for the enum constants, I came accross an error called 'Illegal forward reference', which made me have to find a different way to access the DnaComplement nucleotides. I also noticed what you mentioned about the spellings, the shortTitle of GUANINIE and CYTOCINE didn't match their respective nucleotide.

I have to acknowledge this program is way better than the first one. Here's the improved program:

import java.util.ArrayList;
import java.util.List;

public class DnaStrand {

private final List<DnaNucleotide> dnaNucleotideSequence;
    private final DnaStrandType dnaStrandType;
    private final RnaStrand complementRnaStrand;

public DnaStrand(List<DnaNucleotide> dnaNucleotideSequence) {
        this(dnaNucleotideSequence, DnaStrandType.TEMPLATE);
    }
    public DnaStrand(List<DnaNucleotide> dnaNucleotideSequence, DnaStrandType dnaStrandType) {
        this.dnaNucleotideSequence = List.copyOf(dnaNucleotideSequence);
        this.dnaStrandType = dnaStrandType;
        complementRnaStrand = new RnaStrand(List.copyOf(dnaStrandType.transcribeDna(dnaNucleotideSequence)),
                                this);
    }

public RnaStrand getComplementRnaStrand() {
        return complementRnaStrand;
    }

public String dnaStrandNucleotideSequenceString() {
        StringBuilder dnaNucleotideSequence = new StringBuilder();
        for (DnaNucleotide nucleotide : this.dnaNucleotideSequence) {
            dnaNucleotideSequence.append(nucleotide.shortTitle());
        }
        return dnaNucleotideSequence.toString();
    }

@Override
    public String toString() {
        return  "%s%n%s%n%s".formatted(dnaStrandType + " STRAND",
                                            "DNA strand: " + "\t\t" + dnaStrandNucleotideSequenceString(),
                                            "RNA complement: " + "\t" +
                                            complementRnaStrand.rnaStrandNucleotideSequenceString());
    }

public static List<DnaNucleotide> toDnaNucleotideList(String s) {
        List<DnaNucleotide> dnaNucleotides = new ArrayList<>();
        char[] nucleotidesArray = s.toCharArray();
        for (char c : nucleotidesArray)
            for (DnaNucleotide n : DnaNucleotide.values())
                if (Character.toUpperCase(c) == n.shortTitle().charAt(0))
                    dnaNucleotides.add(n);

return dnaNucleotides;
    }

public static void main(String[] args) {
        DnaStrandType type = DnaStrandType.valueOf(args[1].toUpperCase());
        DnaStrand dnaStrand = new DnaStrand(toDnaNucleotideList(args[0]), type);
        System.out.println(dnaStrand);
    }
}

public enum DnaNucleotide {

ADENINE("A", RnaNucleotide.URACIL, RnaNucleotide.ADENINE),
    CYTOSINE("C", RnaNucleotide.GUANINE, RnaNucleotide.CYTOSINE),
    GUANINE("G", RnaNucleotide.CYTOSINE, RnaNucleotide.GUANINE),
    THYMINE("T", RnaNucleotide.ADENINE, RnaNucleotide.URACIL);

private final RnaNucleotide templateStrandRnaComplement;
    private final RnaNucleotide codingStrandRnaComplement;
    private final String shortTitle;

private DnaNucleotide(String shortTitle, RnaNucleotide templateStrandRnaComplement,
                          RnaNucleotide codingStrandRnaComplement) {
        this.shortTitle = shortTitle;
        this.templateStrandRnaComplement = templateStrandRnaComplement;
        this.codingStrandRnaComplement = codingStrandRnaComplement;
    }

public DnaNucleotide DnaComplement() {
        return switch (this) {
            case ADENINE -> THYMINE;
            case CYTOSINE -> GUANINE;
            case GUANINE -> CYTOSINE;
            case THYMINE -> ADENINE;
            default -> throw new AssertionError("Unknown nucleotide: " + this);

};
    }

public RnaNucleotide templateStrandRnaComplement() {
        return templateStrandRnaComplement;
    }

public RnaNucleotide codingStrandRnaComplement() {
        return codingStrandRnaComplement;
    }

public String shortTitle() {
        return shortTitle;
    }

}

Campbell Ritchie

Marshal

Posts: 82459

594

posted 2 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

Alexander Santana wrote:. . . Thanks . . . Campbell Ritchie! . . .

That's a pleasure

Sorry for taking so long to reply.

As I said earlier, using an enum makes the switch unnecessary.Find out how to use the valueOf() method to get your nucleotide directly from "A" "C" "T" or "G" (or "U"). You might have to change the names of the enum elements to single letters. Alternatively, you can use a Map<String, XyzNucleotide>.
Agree with your throwing an exception if you don't get the right name, but I don't like AssertionError. I think that exception was developed for the assert keyword which was introduced in Java1.4 by which time lots of books had shown people the following abomination:-I think, only ever declare or catch Exception itself if you really don't know what sort of Exception you are expecting. Using the catch shown above creates a risk that if assert threw a subtype of RuntimeException it might be caught without the programmer knowing the assertion has failed. It might have been better for AssertionError to have package‑private access, rather than public, but after the class has been let loose on an unsuspecting world, that sort of change becomes impossible to implement. I think you should only ever declare, throw or catch Throwable, or Error and its subtypes, if you only want to try them out and see what happens. And if you do, beware of this type which counts as an exception but actually is thrown when a normal event occurs. It appears that latter problem will disappear in the future.

Campbell Ritchie

Marshal

Posts: 82459

594

posted 2 months ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

If you go back to valueOf(), you will find it declares a much more appropriate kind of Exception. If you ever suffer such an Exception, it means you have a corrupt input, e.g. "ACTGAGCTGCTAXACTG" and need to get the correct input before your program should be run. I shall let you work out how to throw such an Exception if you use a Map and get "X" as the input.