5
\$\begingroup\$
  1. I should write a CLI through which two Strings can be read in. These two Strings should be checked for whether they are anagrams of each other
  2. For both Strings, I should give out all their possible anagrams. To avoid repetition, I only give out the anagrams for both of them if they are not already anagrams of each other. Otherwise, I only give out the anagrams for String 1 (since they already include the anagrams for String 2)

My code works without errors (as intended). Now I'm looking on whether I can improve it. Appreciate any help.

Result:

public class Anagram {

static final String SECOND_STRING_INPUT = "You have inserted '%s'. Please insert"
    + " a second String to check whether it's an anagram of '%s'!";

static final String ARE_ANAGRAMS = "\n'%s' and '%s' are anagrams: %b !%n";

private static Set<String> anagramSet;


public static void main(String[] args) {

    Scanner inputScan = new Scanner(System.in);
    
    while(true) {
        
        System.out.println("\nPlease enter a String. \nTo end the program, leave "
            + "the input empty and press enter!");
        String firstInput = inputScan.nextLine();
        
        if(firstInput.equals("")) {
            
            break;
        }
        
        String secondInput = "";
        
        while(secondInput.equals("")) {
            
            System.out.printf(SECOND_STRING_INPUT, firstInput, firstInput);
            secondInput = inputScan.nextLine();
            
            if(secondInput.equals("")) {
                
                System.out.println("The input at this point can not be blank. "
                        + "Please provide a non-blank input.");
            }
        }
        
        performEvaluation(firstInput, secondInput);
        System.out.println("\n---------new evaluation begins here----------");
    }

    System.out.println("The program's execution has been ended!");
    inputScan.close();
}


private static void performEvaluation(String firstInput, String secondInput) {
    
    boolean areAnagrams = areAnagrams(firstInput, secondInput);
    
    System.out.printf(ARE_ANAGRAMS, firstInput, secondInput, areAnagrams);
    
    if(firstInput.replace(" ","").length() <= 10) {
        
        System.out.println(allAnagramMessages(areAnagrams, firstInput, secondInput));
        printAnagrams(firstInput.toLowerCase().replace(" ",""));
    }else {
        System.out.println("The anagrams will not be printed, since the Strings "
                + "were logner than ten letters!");
    }
    
    if(!areAnagrams && !firstInput.equals(secondInput) 
            && secondInput.replace(" ","").length() <= 10) {
        
        System.out.printf("%nThe possible anagrams of '%s' are: %n", secondInput);
        printAnagrams(secondInput);
    }
}


private static void printAnagrams(String i_Word) {
    
    i_Word = stripDownString(i_Word);
    
    anagramSet = new HashSet<>();
    findAnagrams(i_Word);
    anagramSet.forEach(System.out::println);
}


public static boolean areAnagrams(String firstInput, String secondInput) {
    
    String firstInputMod = stripDownString(firstInput);
    String secondInputMod = stripDownString(secondInput);
    
    if(firstInputMod.length() != secondInputMod.length()) {
        
        return false;
    }else if(firstInputMod.equals(secondInputMod)) {
        
        return false;
    }else{
        
        return disassembleToLetters(firstInputMod)
                .equals(disassembleToLetters(secondInputMod));
    }
}


private static Map<String,Integer> disassembleToLetters(String i_word){
    
    Map<String,Integer> resultMap = new HashMap<>();
    
    stringToList(i_word)
        .forEach(lttr -> resultMap.put(lttr, resultMap.getOrDefault(lttr, 0)));
    
    return resultMap;
}


private static String allAnagramMessages(boolean areAnagrams, String firstInput,
        String secondInput) {
    
    StringBuilder sb = new StringBuilder();
    
    sb.append(String.format("%nThe possible anagrams of '%s' ", firstInput));
    
    if(areAnagrams) {
        sb.append(String.format("and '%s' ", secondInput));
    }
    
    sb.append("are:");
    
    return sb.toString();
}


public static void findAnagrams(String i_Word) {
    
    String wordStripped = stripDownString(i_Word);
    
    if(wordStripped.length() == 1) {
        
        anagramSet.add(wordStripped);
        return;
    }
    
    stringToList(wordStripped).stream().forEach(lttr -> {
        List<String> restList = new ArrayList<>(stringToList(wordStripped));
        restList.remove(lttr);
        
        findAnagrams(new StringBuilder(lttr), restList);
    });
}


public static void findAnagrams(StringBuilder currString, List<String> restList) {
    
    if(restList.size() == 1) {
        
        StringBuilder currStringCopy = new StringBuilder(currString);
        anagramSet.add(currStringCopy.append(restList.get(0)).toString());
    }else {
        
        restList.stream().forEach(lttr -> {
            StringBuilder currStringCopy = new StringBuilder(currString);
            currStringCopy.append(lttr);
            
            List<String> restListCopy = new ArrayList<>(restList);
            restListCopy.remove(lttr);
            
            findAnagrams(currStringCopy, restListCopy);
        });
    }
}


private static String stripDownString(String i_Word) {
    
    return i_Word.replace(" ","").toLowerCase();
}


public static Set<String> getAnagramSet(){
    
    return anagramSet;
}


public static void setAnagramSet(Set<String> i_Set){
    
    anagramSet = i_Set;
}

}

\$\endgroup\$
1
  • \$\begingroup\$ Method stringToList() is missing \$\endgroup\$ Commented Oct 14 at 10:05

2 Answers 2

2
\$\begingroup\$

readability, maintainability

why do you have SECOND_STRING_INPUT but no FIRST_STRING_INPUT ? its good to declare constants for literals (for example, it will be easier to make the application multi-language) - be consistent.

firstInput.equals("") : there are multiple places in the code where you ask if a string is or is not empty, so what if you will want to change the quit condition? you will need to change multiple places. better make one method. boolean isQuit(String input) and you have the bonus of giving description what does it mean that the input is empty

on the same topic, the length limit (10) is also specified multiple times. put inside a constant.

language features

instead of Strings built from concatenated multi lines, use text blocks

you can use try-with-resources to handle open and close of the scanner. it ensures the IO resource is closed, even in cases of exceptions.

best practices

going back to firstInput.equals("") : putting the variable on left side may produce null pointer exception, as best practice, put the literal on the left side "".equals(firstInput) another option is to use Apache commons lang StringUtils class, which allows null safe checks and operations on Strings (including replace)

\$\endgroup\$
0
2
\$\begingroup\$

Single Responsibility Principle

Your code violates SRP (the first principle of SOLID).

Instead, piling all the code into the Anagram class, delineate the responsibilities you have in this application and introduce separate types representing these responsibilities.

There are three separate areas of responsibility in your code:

  • UI, interaction with standard input and standard output;

  • Checking if two strings are anagrams;

  • Generating all permutations of characters in a given string.

Avoid mutating static State

Manipulations with a static state are error-prone, and usually it's an indicator of a bad design.

Instead of updating static field anagramSet you can introduce a class responsible for generating character permutations (as I suggested above) extracting all relevant functionality into it and turning this variable into an instance field.

stream().forEach()

When you see the following pattern in the code

collection.stream().forEach()

that's clearly an abuse of the Stream API because:

  1. You don't need a stream in such cases. Instead, you should be using Iterable.forEach() or enhanced for-loop (sometimes called for-each loop).

  2. Functional programming paradigm favors functions pure producing a result over functions operating through side-effects. When you're trying to apply Stream.forEach(), pause for a second, because it's likely that you're doing something wrong (there might a better way to achieve the same result, and you might not need streams at all). The Stream API documentation warns that operations performing side-effects should be used cautiously.

Iterable.forEach() will not buy much in this case, because multiline lambda expression are not very readable. I would advise refactoring the stream in the findAnagrams() method into an enhanced for-loop.

Don't treat streams as fancy loops. These are different constructs.

Naming

Names i_Word, i_Set are not aligned with the Java language naming convention. And besides that, they are not descriptive.

Method name disassembleToLetters() is misleading. It rather suggests splitting a string into separate characters than generating a Map of character frequencies.

Removing Whitespace / Clear prompts

Your application has a hidden assumption that whitespace is not taken into account, which it doesn't communicate to the user. Consider refining the prompts.

And if your intention is to remove from the input string not only " " but also other forms of whitespace, then instead of:

.replace(" ", "")

you have to use at least this regular expression

.replaceAll("\\s+", "")

Note that matches only whitespace characters in the ASCII area (i.e. [\t\n\x0B\f\r]). But it will be probably sufficient for the needs of your application, since your code can't dial with code-points and other Unicode construct (but feel free to investigate this topic further if you're interested).

Also, firstInput.equals("") is the exact equivalent of firstInput.isEmpty() which is more readable.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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