3
\$\begingroup\$

Purpose

This problem comes from this dailyProgrammer subreddit challenge.

The task is to take an input of C / cs and V / vs and to replace Cs with random consonants and Vs with random vowels (English alphabet) while keeping the case the same. Y is not considered a vowel in this case.

Example:

  • CcVvv => BgAoi
  • ccVVV => zqUUE

Implementation

For this case, I've put all methods in the same class - in reality, I might split these methods out into different classes.

Edit: I forgot to throw an exception in generateRandomString when the isValidCharacter method is called and returns false.

public class RandomCharacterReplacer {
  public static final char C = 'C';
  public static final char V = 'V';
  public static final List<Character> CONSONANTS = new ArrayList<>(
      Arrays.asList('B', 'C', 'D', 'F', 'G', 'H', 'J', 'K', 'L', 'M', 'N', 'P', 'Q', 'R', 'S', 'T', 'V', 'W', 'X', 'Y', 'Z')
  );
  public static final List<Character> VOWELS = new ArrayList<>(
      Arrays.asList(
          'A', 'E', 'I', 'O', 'U'
      )
  );

  private static boolean isValidCharacter(final char c) {
    final char upperCaseChar = Character.toUpperCase(c);
    return upperCaseChar == C || upperCaseChar == V;
  }

  public static AlphabetCharacterCase returnAlphabetCharacterCase(final char c) {
    if (!Character.isAlphabetic(c)) {
      throw new RuntimeException("character is non-alphabetic");
    }

    if (Character.isUpperCase(c)) {
      return AlphabetCharacterCase.UPPER;
    }

    if (Character.isLowerCase(c)) {
      return AlphabetCharacterCase.LOWER;
    }

    throw new RuntimeException("unexpected character case");
  }

  public static AlphabetCharacterType returnAlphabetCharacterType(final char c) {
    if (!Character.isAlphabetic(c)) {
      throw new RuntimeException("character is non-alphabetic");
    }

    final char upperCaseChar = Character.toUpperCase(c);
    if (upperCaseChar == C) {
      return AlphabetCharacterType.CONSONANT;
    }

    if (upperCaseChar == V) {
      return AlphabetCharacterType.VOWEL;
    }

    throw new RuntimeException("unexpected character type");
  }

  public static char returnRandomCharacter(final List<Character> characterList) {
    if (characterList.isEmpty()) {
      throw new IllegalArgumentException("character list must be non-empty");
    }
    final Random random = new Random();
    return characterList.get(random.nextInt(characterList.size() - 1));
  }

  public static List<Character> returnAlphabetCharacterTypeList(final AlphabetCharacterType characterType) {
    switch (characterType) {
      case CONSONANT: {
        return CONSONANTS;
      }

      case VOWEL: {
        return VOWELS;
      }

      default: {
        throw new RuntimeException("unexpected character type");
      }
    }
  }


  public static char generateRandomCharacter(final AlphabetCharacterCase alphabetCharacterCase, final AlphabetCharacterType characterType) {
    final List<Character> characters = returnAlphabetCharacterTypeList(characterType);
    switch (alphabetCharacterCase) {
      case UPPER: {
        return returnRandomCharacter(characters);
      }

      case LOWER: {
        return Character.toLowerCase(returnRandomCharacter(characters));
      }

      default: {
        throw new RuntimeException("unexpected character case");
      }
    }
  }

  public static String generateRandomString(final String input) {
    final char[] chars = input.toCharArray();
    final StringBuilder stringBuilder = new StringBuilder();
    for (final char c : chars) {
      if (isValidCharacter(c)) {
        final AlphabetCharacterCase alphabetCharacterCase = returnAlphabetCharacterCase(c);
        final AlphabetCharacterType alphabetCharacterType = returnAlphabetCharacterType(c);
        stringBuilder.append(generateRandomCharacter(alphabetCharacterCase, alphabetCharacterType));
      }
    }

    return stringBuilder.toString();
  }
}
\$\endgroup\$
4
\$\begingroup\$

I think if I were doing this, I'd do it...somewhat (a lot?) differently. My immediate reaction would be to do something on this general order (using C++ syntax instead of Java, but I'm pretty sure the same basic idea should work in Java about the same way):

char gen_rand(std::string const &input) { 
    // A quick and dirty method, for now.
    return input[rand() % input.size()];
}    

static const string lower_c = "bcdfghjklmnpqrstvwxyz";
static const string upper_c = "BCDFGHJKLMNPQRSTVWXYZ";
static const string lower_v = "aeiou";
static const string upper_v = "AEIOU";

string genRandomString(std::string const &input) { 
    string output; // I suppose needs to be a StringBuilder in Java

    for (char ch : input)
        switch (ch) {
        case 'c': output.push_back(gen_rand(lower_c));    break;
        case 'C': output.push_back(gen_rand(upper_c));    break;
        case 'v': output.push_back(gen_rand(lower_v));    break;
        case 'V': output.push_back(gen_rand(upper_v));    break;
        default: throw std::runtime_error("Unexpected character in input");
        }
     return output;
 }

If I'm allowed to speak in generalities, your code seems (to me) to spend a lot of effort on issues that are almost incidental to the question at hand (e.g., 37 lines just to decide that c, C, v and V refer to lower- and upper-case consonants and vowels). At least as I see things, the fact that the case of the input corresponds to the case of the output is really mostly incidental--they could just as well be a, b, c and d instead.

Another option that seems obvious to me, would be to use a map to take an input character and retrieve the collection of characters to choose from for that input. This is probably more work than it's worth for only 4 fixed inputs, but if you might have a lot of inputs, or (especially) if you want to support those inputs being specified at run time (e.g., reading them from a configuration file) a map becomes much more attractive.

Bottom line: it's a lot simpler and more flexible to just treat the mapping from input character to action as entirely arbitrary rather than go to a lot of work to classify the inputs as upper/lower case.

\$\endgroup\$
2
  • \$\begingroup\$ thanks for the input -- you're absolutely correct. I think one of the challenges for me is that I tend to over-complicate my code / approach. Do you have any strategies you use to identify places where your code might be more complicated than it needs to be? \$\endgroup\$ – Jae Bradley Feb 9 '16 at 3:01
  • \$\begingroup\$ @JaeBradley: I don't really have what I'd call particularly good or dependable strategies. To the extent I have a strategy at all, it basically comes down to thinking about whether there's a way I can be lazier, and have a good chance that it won't be a matter of being lazy now that'll lead to drastically more work later (e.g., failing to check for errors). \$\endgroup\$ – Jerry Coffin Feb 9 '16 at 5:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.