Is there a better alternative or way to improve code?

Question

I have a string which has uppercase & lowercase letters with numbers and @ symbol which I have split up into individual elements. The goal here was to create this simple brute force code that cycles through the string and returns the element given the counter. It's inefficient and slow.

    private static String allCharacters =
          "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890@";

    public static void main(String[] args) {

    String pass;
    //split into individual letters
    String[]text = allCharacters.split("");

    for (int counter = 0; counter < text.length; counter++) {
        for (int c2  = 0; c2 < text.length; c2++) {
            pass = text[counter] + text[c2];
            System.out.println(pass);
        }
    }
}

I also want to add a certain length instead of using multiple for loops, Any tips ?

What is the element and which counter is the point of concern? — nice_dev, Aug 01 '20 at 20:56
Sorry, but I do not understand the algorithm. What exactly does it compute? — Turing85, Aug 01 '20 at 20:57
If you want to print all possible two-character combinations, then using two loops is the correct approach. You could bake it into a single loop, and then do some division and modulo on the counter to separate it into an "outer" and "inner" counter, but that a) doesn't make your code faster and b) does make your code less readable, so don't. — , Aug 01 '20 at 20:59
this program outputs AA - AZ, then Aa - Az, A1- A@, its a very simplistic password guesser — GreyShinobi, Aug 01 '20 at 21:17
The question - as is -is hard to answer without supplying a complete solution. For one, you may want to look at the concept of [recursion](https://en.wikipedia.org/wiki/Recursion). This will lead to a readable, but performance- and memory-wise suboptimal solution. For another, one can proof that [all recursions can be rewritten as iterations and vice-versa](https://stackoverflow.com/questions/2093618/can-all-iterative-algorithms-be-expressed-recursively). Since iterations are normally faster, in performance-critical applications, it may be worth wile to transform recursions to iterations. — Turing85, Aug 01 '20 at 22:24
You claim this is slow. Do you have any evidence to back up your claim? — NomadMaker, Aug 01 '20 at 22:57

score 0 · Answer 1 · answered Aug 01 '20 at 21:28

0

If you want to combine more than 2 characters, it is likely better to use recursion rather than a loop:

public void generatePossibilities(String prefix, int targetLength) {
    if (prefix.length() == targetLength) {
         System.out.println(prefix);
         return;
    }
    
    generatePossibilities(prefix + "a", targetLength);
    generatePossibilities(prefix + "b", targetLength);
    // you get the idea. Of course, you should use a loop for this
    // this is left as an exercise to the reader
}

Two side notes:

a) Password brute-forcing is supposed to be slow. If this problem didn't have exponential complexity, passwords would be useless.

b) Your performance bottleneck is most likely System.out.println. Try dumping the passwords into a file instead, that is likely faster than your terminal.

answered Aug 01 '20 at 21:28

If you are not sure what OP wants, you should ask for clarification in the comments. --- Why should recursion be preferred over iteration? As far as my knowledge goes, loops are in general faster and less memory-intensive. – Turing85 Aug 01 '20 at 22:08
@Turing85, OP already clarified. IMO recursion is just the more natural pattern for expressing this kind of thing. – Aug 01 '20 at 22:17
... and slower and more memory-intensive. This should be mentioned. – Turing85 Aug 01 '20 at 22:19
But not to an extent that is likely to be relevant or noticeable in this context. – Aug 01 '20 at 22:33

score 0 · Accepted Answer · answered Aug 01 '20 at 22:08

Your code now generates all combinations of characters from a given set. I think the use of String.charAt(int index) is more compact.

for two-letter words it looks like this:

for (int i = 0; i < allCharacters.length(); i++) {
    for (int j = 0; j < allCharacters.length(); j++) {
    System.out.println(allCharacters.charAt(i) +""+ allCharacters.charAt(j));
    }
}

if you need performance, indeed leave out the System.out.println it is very slow. Also you may want to rewrite to ++i instead of i++, that makes a little difference

If you are really concerned with benchmarks, you would also replace the temporary String object and work directly on an char[] like this

    char[] all = allCharacters.toCharArray();
    char[] word = new char[2];

    for (int i = 0; i < all.length; ++i) {
        for (int j = 0; j < all.length; ++j) {
            word[0] = all[i];
            word[1] = all[j];
            //System.out.println(word);
        }
    }

If you need even more speed, then make a lookup table. That means, write all words into the memory once (again as an array), there they are accessible much faster for the rest of your program's execution than generating them on the fly. Lookup is of course limited by available memory.

But the real question is: is brute-force really necessary? For that to answer, we would need to understand more about the underlying problem.

Is there a better alternative or way to improve code?

2 Answers2