HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

Regularity in the "Rusty Towel of Mutual understanding"

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rustyregularitytheunderstandingmutualtowel

Problem

I have the following Java class:

import java.util.Random;

public class RandomNameGenerator {

    private Random rand;

    private static String[] prefixes = { "Ultimate", "Bloody", "Crooked",
            "Hallowed", "Magnificent", "Heavy", "Jagged", "Grand", "Shiny",
            "Rusty" };

    private static String[] items = { "Chainsaw", "Towel", "Ping-Pong Ball",
            "Longsword", "Scissors", "Dagger", "Blade", "Bow", "Axe", "Dagger",
            "Spoon", "Fork", "Coat", "Chain Mail", "Plate Mail", "Cloak",
            "Cape", "Mirror", "Cauldron", "Pouch", "Boots", "Shoes", "Greaves",
            "Pants", "Robes", "Locket", "Ring", "Amulet", "Potion", "Fish",
            "Teapot", "Hood", "Crown", "Cap", "Helmet" };

    private static String[] addons = { "Glorious", "Bloody", "Prolonged",
            "Bitter", "Wicked", "Furious" };

    private static String[] postfixes1 = { "Destruction", "Feminism",
            "Twilight", "Massacre", "Dread", "Terror", "Mutual Understanding",
            "Spite", "Immobility", "Mediocrity", "Anger" };

    private static String[] postfixes2 = { "the Occult", "the Captain",
            "the Warrior", "the Hunter", "the Haunted", "the Dead",
            "the Fallen", "the Hitchhiker", "the Wicked King", "the Grue" };

    public RandomNameGenerator() {
        rand = new Random(System.currentTimeMillis());
    }

    public String getRandomName() {
        StringBuilder str = new StringBuilder();
        if (rand.nextInt(100)  postfixes2.length) {
            if (rand.nextInt(100) < 70) {
                str.append(addons[rand.nextInt(addons.length)]);
                str.append(" ");
            }
            str.append(postfixes1[rand.nextInt(postfixes1.length)]);
        } else {
            str.append(postfixes2[rand.nextInt(postfixes2.length)]);
        }
        return str.toString();
    }
}


I use it in a small program to generate sample item names like


Shiny Dagger of the Haunted

Shoes of Im

Solution

Your randomness for calculating the last postfixes is not regular.... there are very slight deviations

(Note update on the doubling of daggers at the end though... that's not 'slight').

Let's do a couple of things here. Firstly, rename them:

postfixes1 -> concepts:

private static String[] concepts = { "Destruction", "Feminism",
        "Twilight", "Massacre", "Dread", "Terror", "Mutual Understanding",
        "Spite", "Immobility", "Mediocrity", "Anger" };


postfixes2 -> people:

private static String[] people = {"the Occult", "the Captain",
        "the Warrior", "the Hunter", "the Haunted", "the Dead",
        "the Fallen", "the Hitchhiker", "the Wicked King", "the Grue"};


Then, you can make the logic clearer near the end. Your code is (with the renames):

if (rand.nextInt(concepts.length + people.length) > people.length) {
        if (rand.nextInt(100) < 70) {
            str.append(addons[rand.nextInt(addons.length)]);
            str.append(" ");
        }
        str.append(concepts[rand.nextInt(concepts.length)]);
    } else {
        str.append(people[rand.nextInt(people.length)]);
    }


Now, what's the problem there?

Well, there are 2 ...

-
people should not have 'the' prepended to all of them. It is repeating yourself. Instead you should have:

str.append("the ").append(people[rand.nextInt(people.length)]);


-
there is an off-by-one error in if (rand.nextInt(concepts.length + people.length) > people.length) {. This is hard to explain... but....

nextInt(limit) selects a random number up to, but not including the limit.

in this case, the limit is concepts.length + people.length, and, if you put those two arrays together, there are 21 members (11 concepts, 10 people). So, you select a random number up to, but not including 21... then, if it is greater than people.length (10), you choose to add a concept. So, the chances are 10 in 21 that it will be > 10 (values 11, 12, 13, ... 20), and then you chose to do the side that has 11 entries. You are using the proportions of people to decide whether to do a concept.....

Your code should be:

if (rand.nextInt(concepts.length + people.length) >= people.length) {
    // add a concept


or, more logically:

if (rand.nextInt(concepts.length + people.length) < concepts.length) {
    // add a concept


So, you do have an imbalance. You are selecting people about 10% more often than you should....

Update: Jeroen and I were discussing some odd values coming out with dagger, and I hacked out some of your code, and ran it through with this loop:

public static void main(String[] args) {
    RandomNameGenerator rng = new RandomNameGenerator();
    Map counts = new TreeMap<>();
    for (int i = 0; i  System.out.printf("%6d %s%n", v.get(), k));
}


Note, what I am doing is just counting the occurrences of a String.

I then stripped out some of the 'qualifiers' from your code, and got results like:

110 Amulet of Anger
   126 Amulet of Destruction
   114 Amulet of Dread
   ......
   127 Chain Mail of Feminism
   125 Chain Mail of Immobility
   118 Chain Mail of Massacre
   134 Chain Mail of Mediocrity
   ......
   140 Crown of the Hitchhiker
   154 Crown of the Hunter
   168 Crown of the Occult
   155 Crown of the Warrior
   141 Crown of the Wicked King
   243 Dagger of Anger
   233 Dagger of Destruction
   271 Dagger of Dread
   245 Dagger of Feminism
   245 Dagger of Immobility
   240 Dagger of Massacre
   254 Dagger of Mediocrity
   ...
   318 Dagger of the Hunter
   299 Dagger of the Occult
   328 Dagger of the Warrior
   295 Dagger of the Wicked King
   131 Fish of Anger
   130 Fish of Destruction
   101 Fish of Dread
   129 Fish of Feminism


Note how Dagger appears about twice as often as other values?

That's because you have "Dagger" in the input data twice:

private static String[] items = { .....,
        "Longsword", "Scissors", "Dagger", "Blade", "Bow", "Axe", "Dagger",
        //
        // Dagger twice          ^^^^^^^^                         ^^^^^^^^
        ..... 
        "Teapot", "Hood", "Crown", "Cap", "Helmet" };

Code Snippets

private static String[] concepts = { "Destruction", "Feminism",
        "Twilight", "Massacre", "Dread", "Terror", "Mutual Understanding",
        "Spite", "Immobility", "Mediocrity", "Anger" };
private static String[] people = {"the Occult", "the Captain",
        "the Warrior", "the Hunter", "the Haunted", "the Dead",
        "the Fallen", "the Hitchhiker", "the Wicked King", "the Grue"};
if (rand.nextInt(concepts.length + people.length) > people.length) {
        if (rand.nextInt(100) < 70) {
            str.append(addons[rand.nextInt(addons.length)]);
            str.append(" ");
        }
        str.append(concepts[rand.nextInt(concepts.length)]);
    } else {
        str.append(people[rand.nextInt(people.length)]);
    }
str.append("the ").append(people[rand.nextInt(people.length)]);
if (rand.nextInt(concepts.length + people.length) >= people.length) {
    // add a concept

Context

StackExchange Code Review Q#64402, answer score: 34

Revisions (0)

No revisions yet.