Validating parameters according to a fixed reference

Question

The following method is for setting the transfer type of an FTP connection. Basically, I'd like to validate the character input (see comments).

Is this going overboard? Is there a more elegant approach? How do you approach parameter validation in general? Any comments are welcome.

public void setTransferType(Character typeCharacter,
        Character optionalSecondCharacter) throws NumberFormatException,
        IOException {

    // http://www.nsftools.com/tips/RawFTP.htm#TYPE
    // Syntax: TYPE type-character [second-type-character]
    //
    // Sets the type of file to be transferred. type-character can be any
    // of:
    //
    // * A - ASCII text
    // * E - EBCDIC text
    // * I - image (binary data)
    // * L - local format
    //
    // For A and E, the second-type-character specifies how the text should
    // be interpreted. It can be:
    //
    // * N - Non-print (not destined for printing). This is the default if
    // second-type-character is omitted.
    // * T - Telnet format control (<CR>, <FF>, etc.)
    // * C - ASA Carriage Control
    //
    // For L, the second-type-character specifies the number of bits per
    // byte on the local system, and may not be omitted.

    final Set<Character> acceptedTypeCharacters = new HashSet<Character>(Arrays.asList(
            new Character[] {'A','E','I','L'}
    ));

    final Set<Character> acceptedOptionalSecondCharacters = new HashSet<Character>(Arrays.asList(
            new Character[] {'N','T','C'}
    ));

    if( acceptedTypeCharacters.contains(typeCharacter) ) {
        if( new Character('A').equals( typeCharacter ) || new Character('E').equals( typeCharacter ) ){
            if( acceptedOptionalSecondCharacters.contains(optionalSecondCharacter) ) {
                executeCommand("TYPE " + typeCharacter + " " + optionalSecondCharacter );
            }
        } else {
            executeCommand("TYPE " + typeCharacter );
        }
    }
}

Answer 1

Here's how I would go about it:

private static final List<String> ALLOWED = Arrays.asList("AN", "AT", "AC", "EN", "ET", "EC");

public void setTransferType(Character type, Character optional) 
  throws NumberFormatException, IOException {

  String[] arr = new String[] { "AN", "AT", "AC", "EN", "ET", "EC" };

  if(type = 'I')
     executeCommand("TYPE " + type );
  else if(type = 'L') {
     executeCommand("TYPE " + type + " " + optional);
  else if(ALLOWED.contains("" + type + optional)) 
     executeCommand("TYPE " + type + " " + optional);
  else
     // Handle an incorrect argument error               
}

• Your code did not handle the 'L' case. I assumed you want it do executeCommand("TYPE " + type + " " + optional);
• Your code silently ignored incorrect parameters. I added an else (the last one) for explicitly handling this case.
• The names that you use are too noisy. For instance, You declare a variable of type Character and you name it typeCharacter. The 'Character` suffix of the name is noise - it does not add any useful information.
• If your method's interface and responsibilities are clear then I would not worry too much about it implementation (esp. in such short methods). Just write the simplest thing that makes it passes it tests. If you haven't written one yet, they you'd better start right away. You'd be better off spending your time writing it instead of polishing the implementation.

Answer 2

Validate the entire string in one simple regex check:

String.matches("TYPE ([AE] [NTC]|I|L .)")

(Note that I used "." (any character) after the L because the doc did not explain what that character should be. If it can only be, say, 7 or 8, use [78].)

You can go further with this (capturing groups, precompiling the regex, allowing arbitrary extra whitespace, etc.) but the above basically does it.