5
\$\begingroup\$

This code will generate a pseudorandom alphanumeric string of given length. I would welcome suggestions on how to make it more random. Beyond that, have I made any convention violations, Exception cases, and the like? Also, is there any way to make it faster?

public class Test
{
    public static String getRandomAlphaNum(int length)
    {
        String charstring = "abcdefghijklmnopqrstuvwxyz0123456789";
        String randalphanum = "";
        double randroll;
        char randchar;
        for
        (double i = 0; i < length; i++)
        {
            randroll = Math.random();
            randchar = '@';
            for
            (int j = 1; j <= 36; j++)
            {
                if
                (randroll <= (1.0 / 36.0 * j))
                {
                    randchar = charstring.charAt(j - 1);
                    break;
                }
            }
            randalphanum += randchar;
        }
        return randalphanum;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Use StringBuilder to concatenate strings. += Creates new string every time and it is expensive operation to do in a loop \$\endgroup\$ – purple Dec 7 '19 at 17:15
  • 1
    \$\begingroup\$ Please do not use all caps... that would indicate that each letter stands for something (an acronym). That is not the case with Java \$\endgroup\$ – JoelFan Dec 8 '19 at 1:08
  • 1
    \$\begingroup\$ using double as type of the counter variable is dangerous because primitive floating point numbers cannot be represented exactly (when they have fractions). Avoid primitive floating point types unless accuracy is less important the speed in your calculations. \$\endgroup\$ – Timothy Truckle Dec 8 '19 at 12:42
7
\$\begingroup\$
for
        (double i = 0; i < length; i++)

And related loops should have the "for" on the same line (as this is a common coding convention.)

(randroll <= (1.0 / 36.0 * j))

This doesn't have to be a double; instead, the random number can be generated as an integer (to select which element from the array.)

randchar = '@';

Unless the random string is not random, I would not initialize the variables with sample data. I'd just leave them blank and then adjust the loop to always run at least once (a do-while loop) so that it becomes initialized.

for
            (int j = 1; j <= 36; j++)
            {
                if
                (randroll <= (1.0 / 36.0 * j))
                {
                    randchar = charstring.charAt(j - 1);
                    break;
                }
            }

I would remove the inner if-statement and un-hardcode the values so it can work with strings with any size. Applying these suggestions, it can be simplified to:

import java.util.Random;
class Main {
  public static void main(String[] args) {
    int strLen = 100;
    String randString = "";
    Random r = new Random();
    String[] chars = "abcdefghijklmnopqrstuvwxyz0123456789".split("");
    while (randString.length() < strLen)
        randString += chars[randBetween(r, 0, chars.length - 1)];

    System.out.println(randString);
  }

  /*
  Generates a random number from min to max inclusive
  */
  public static int randBetween(Random r, int min, int max) {
    return r.nextInt((max - min) + 1) + min;
  }
}

This approach is not optimal as the string is constantly being appended to, meaning that the string has to be re-copied every iteration.

Java introduced Streams, which allows reading forever from certain generators. Knowing this, we can read a stream of random numbers up until the string length that the user wants, and then get the character at the random string length:

import java.util.Random;
class Main {
  public static void main(String[] args) {
    int strLen = 100;
    String chars = "abcdefghijklmnopqrstuvwxyz0123456789";

    StringBuilder randomOutput = new StringBuilder();
    new Random().ints(strLen, 0, chars.length())
                .forEach(c -> randomOutput.append(chars.charAt(c)));

    System.out.println(randomOutput);
  }
}

StringBuilder is used to append the random character as it doesn't have to be re-copied for every loop iteration.

| improve this answer | |
\$\endgroup\$
6
\$\begingroup\$

Separate random generator

I would either extract the random number generator into an extra method, or simply use new Random().nextInt(36) from package java.util to generate a random integer between 0 and 35 (both inclusive).

You could also make the method more generic by adding boundary parameters (min, max). So you can reuse within other limitations.

See: Math.random() explanation

Variable names

Typical Java convention would name things using Camel-case. Also following Cleancode would put as much meaning into their names.

So variables (except simple loop counters) can be renamed:

  • characterOptions or possibleCharacters or alphaNumericChars
  • randomCharacterChoice or randomCharIndex
  • randomString or randomAlphaNumericSequence
| improve this answer | |
\$\endgroup\$
5
\$\begingroup\$

here is my recommendation for your code.

1) Use int instead of a double in the loop; the integer takes less memory than the double.

    //[...]
    char randchar;
    for (int i = 0; i < length; i++) {
        //[...]
    }

2) Use a StringBuilder to accumulate the result, instead of a string + concatenation (randalphanum). The StringBuilder is always a better choice when building string in a loop.

    //[...]
    String randalphanum = "";

3) Create one constant to hold the possible values, as a char array instead of using "charAt" on a string; the computation will be the same, but in my opinion, this will make the code shorter and more readable.

    public static final char[] CHARSTRING = {
            'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 
            'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 
            'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' }; //(3)

    //[...]
    public static String getRandomAlphaNum(final int length) {
        final StringBuilder randalphanum = new StringBuilder(); //(2)
        double randroll;
        char randchar;
        for (int i = 0; i < length; i++) { //(1)
            randroll = Math.random();
            randchar = '@';
            for (int j = 1; j <= 36; j++) {
                if (randroll <= (1.0 / 36.0 * j)) {
                    randchar = CHARSTRING[j - 1]; //(3)
                    break;
                }
            }
            randalphanum.append(randchar);
        }
        return randalphanum.toString();
    }

Potential refactor

A) Instead of using the for loop with index and not using it, I suggest that you use a "while" loop, and decrement the index in the loop.

while (length > 0) {
    length--;
}

B) Instead of calculating the position, you can generate a random int, in the range [0, 26]; using the java.util.Random#nextInt(int) method.

RANDOM.nextInt(CHARSTRING.length); // between 0 and 25

Complete example:

    public static final char[] CHARSTRING = {
            'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0',
            '1', '2', '3', '4', '5', '6', '7', '8', '9' };

    public static final Random RANDOM = new Random();

    public static void main(final String[] args) {
        System.out.println(getRandomAlphaNum(15));
    }

    public static String getRandomAlphaNum(int length) {

        final StringBuilder accString = new StringBuilder();

        while (length > 0) { //(A)
            final int selectedPosition = RANDOM.nextInt(CHARSTRING.length); //(B)
            accString.append(CHARSTRING[selectedPosition]);
            length--; //(A)
        }

        return accString.toString();
    }
```
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Great first post, good job! \$\endgroup\$ – IEatBagels Dec 9 '19 at 14:44
1
\$\begingroup\$

Here is my solution to your problem, I used a StringBuilder so I didn't have to create a new string and then, used an array of chars to make it super easy to get the value I wanted. The code would probably be slightly more readable if you created the character array by hand but I was lazy today

public static String getRandomALphaNum(int length) {
    char[] charArr = new char[36];
    String alpha = "abcdefghijklmnopqrstuvwxyz0123456789";
    StringBuilder output = new StringBuilder();
    for (int i = 0; i < alpha.length(); i++) {
        charArr[i] = alpha.charAt(i);
    }
    Random r = new Random();
    int randRoll;
    for (int i = 0; i < length; i++) {
        randRoll = r.nextInt(charArr.length);
        output.append(charArr[randRoll]);
    }
    return output.toString();
}
| improve this answer | |
\$\endgroup\$

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.