我有以下Java类:

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) < 10) {
            str.append("+");
            str.append(Integer.toString(rand.nextInt(10) + 1));
            str.append(" ");
        }
        if (rand.nextInt(100) < 50) {
            str.append(prefixes[rand.nextInt(prefixes.length)]);
            str.append(" ");
        }
        str.append(items[rand.nextInt(items.length)]);
        str.append(" of ");
        if (rand.nextInt(postfixes1.length + postfixes2.length) > 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();
    }
}


我在一个小程序中使用它来生成示例项目名称,例如


Shiny闹鬼的匕首
不动之靴
平庸之弓


总的来说,我得到了想要的结果,但是我不禁注意到某些组合非常频繁地弹出,例如


神秘学的乒乓球


经常出现。
导致此结果的设计缺陷?是否有更好的方式格式化我要实现的目标?

评论

+1使诸如“相互理解的生锈毛巾”之类的东西

@Nit我认为它是代码审查的主题,因为您想审查代码并提供产生随机名称的更好方法。您的输出正确,一切正常!

乒乓球并不完全适合那里……就像穿皮大衣,青铜茶壶或机车夹克代替夹克一样。您会用字符串的长度来做很多事情,难道是因为它的长度比其他字符串长,所以该字符串的重量更大吗?
@Nit是否有可能将其移植到其他语言(如德语)中,其中形容词的正确形式取决于其描述的内容?

@Christoph我看不到这种情况。但是,如果您想自己做,请继续。

#1 楼

您计算最后一个后缀的随机性是不规律的。...有很小的偏差

(注意,最后将匕首加倍的更新是……不是“轻微”)。 br />
让我们在这里做几件事。首先,将它们重命名为:

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"};


然后,您可以在结尾处使逻辑更清楚。您的代码是(带有重命名):


    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)]);
    }



现在,问题出在哪里?

,这里有2个...




人们不应该在所有人之前都加上“ the”。它在重复你自己。相反,您应该具有:

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



if (rand.nextInt(concepts.length + people.length) > people.length) {中存在一个错误的错误。这很难解释...但是....

nextInt(limit)选择一个随机数,但不包括限制。

在这种情况下,限制为concepts.length + people.length,如果将这两个数组放在一起,则有21个成员(11个概念,10个人)。因此,您选择一个随机数,最多不超过21,但不包括21 ...然后,如果它大于people.length(10),则选择添加一个概念。因此,在21中有10个机会是> 10(值11、12、13,... 20),然后您选择执行具有11个条目的边。您正在使用人员的比例来决定是否执行概念。....

您的代码应该是:

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


或,更合乎逻辑:

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



因此,您确实存在失衡。您选择人员的频率比您应选择的要多10%。...

更新:Jeroen和我正在讨论匕首产生的一些奇数值,然后我破解了一些代码,然后运行了请注意,我在做的只是计算一个字符串的出现。然后,我从您的代码中删除了一些“限定词”,得到的结果如下: />
那是因为您在输入数据中两次有“匕首”:

public static void main(String[] args) {
    RandomNameGenerator rng = new RandomNameGenerator();
    Map<String, AtomicInteger> counts = new TreeMap<>();
    for (int i = 0; i < 100000; i++) {
        String val = rng.getRandomName();
        if (!counts.containsKey(val)) {
            counts.put(val, new AtomicInteger());
        }
        counts.get(val).incrementAndGet();
    }
    counts.forEach((k,v) -> System.out.printf("%6d %s%n", v.get(), k));
}


评论


\ $ \ begingroup \ $
关于匕首的注解更新.....
\ $ \ endgroup \ $
–rolfl
2014年10月1日15:58

\ $ \ begingroup \ $
这是一个很好的收获,非常感谢您的投入。我已经合并了您建议的更改。
\ $ \ endgroup \ $
– Etheryte
2014年10月1日在20:22

\ $ \ begingroup \ $
我认为在所有人身上都带有“ the”是一个好主意,以防万一他想添加“ Morgan Freeman”。
\ $ \ endgroup \ $
– tobyink
14-10-3在8:51

\ $ \ begingroup \ $
@tobyink最好重命名类别,并将Morgan Freeman放在第一位。照原样,两者之间的有效区别似乎是第二组需要定冠词,而第一组不需要。如果添加了需要不同文章的其他选项,请进一步划分类别:一些“漂亮公主”的锯齿鱼或一些“被遗忘的国家”的“终极汤匙”
\ $ \ endgroup \ $
– Mindor先生
2014年10月3日19:02

#2 楼

prefixes[rand.nextInt(prefixes.length)]


您的代码中多次出现此模式,这是此行中的一个小代码重复,因为prefixes重复了两次。如果您不小心更改了其中一个而不是另一个,则最终会得到一个主要的ArrayIndexOutOfBoundsException

我认为这足以保证方法的提取。 (这是我几年前写的实际方法的略微修改形式。我发现它非常有用)

空元素的数量,尽管您总是使用相同的random变量,但是您的方法可以是: />
public static <E> E randomElement(E[] list, Random random) {
    if (list == null || list.length == 0) {
         return null;
    }
    return list[random.nextInt(list.length)];
}



public <E> E randomElement(E[] list) {
    return list[rand.nextInt(list.length)];
}


随机化具有70%可能性为真的布尔值通常写为

str.append(randomElement(prefixes));
str.append(randomElement(postfixes1));
str.append(randomElement(people)); // with the name change suggestion mentioned by @rolfl


这样可以更容易地看到与确切百分比的直接关系,并且还减少了您对“幻数” 100的依赖。

评论


\ $ \ begingroup \ $
我不会测试列表== null || list.length ==0。这没有任何意义,因为此处的任何使用都会获得非null的非空列表。如果程序更改并且不再生效,您是希望获得Ultimate null还是null还是立即获得异常并修复程序?快速失败,不能容忍任何空值。
\ $ \ endgroup \ $
– maaartinus
2014年10月1日15:58

\ $ \ begingroup \ $
@maaartinus最终的null似乎与程序的其余部分完全一致:)
\ $ \ endgroup \ $
–西蒙·福斯伯格
2014年10月1日16:00

\ $ \ begingroup \ $
感谢您的输入,我已经采纳了您的建议,尤其是公共静态 E randomElement(E [] list,Random random)是一个非常简洁的解决方案。
\ $ \ endgroup \ $
– Etheryte
2014年10月1日在20:19

#3 楼

这可能不是一个错误。更有可能是确认偏差。我错了! @rolfl在列表中两次发现“ Dagger”。分布得很好。


您可以拆分一些东西。现在,您有一个长函数,我不喜欢它。它太大了。

public String getRandomName() {
    StringBuilder str = new StringBuilder();
    if (rand.nextInt(100) < 10) {
        str.append("+");
        str.append(Integer.toString(rand.nextInt(10) + 1));
        str.append(" ");
    }
    if (rand.nextInt(100) < 50) {
        str.append(prefixes[rand.nextInt(prefixes.length)]);
        str.append(" ");
    }
    str.append(items[rand.nextInt(items.length)]);
    str.append(" of ");
    if (rand.nextInt(postfixes1.length + postfixes2.length) > 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();
}


在更高的层次上,它看起来像这样:

public String getRandomName() {
    StringBuilder str = new StringBuilder();
    str.append(getRandomBonusLevel());
    str.append(getRandomPrefix());
    str.append(getRandomItemName());
    str.append(getRandomPostfix());
    return str.toString();
}


具有子功能像

private String getRandomItemName(){
    return items[rand.nextInt(items.length)] + " of ";
}



在较低级别上,您有一些硬编码的东西,例如这些魔术数字:

if (rand.nextInt(100) < 10) {

if (rand.nextInt(100) < 50) {


为什么不将它们存储为常量?

private static final int BONUS_LEVEL_PERCENTAGE_CHANCE = 10;
private static final int PREFIX_PERCENTAGE_CHANCE = 50;


评论


\ $ \ begingroup \ $
并不是真正的机会百分比,返回的数字小于10和/或小于50的机会不是10%或50%。实际上,在中间值范围内的数字百分比要比在极限值范围内的数字百分比高。我希望可以在这里引用一些内容,但是我不记得为什么/如何知道这一点。
\ $ \ endgroup \ $
–马拉奇♦
2014年10月1日15:14

\ $ \ begingroup \ $
@Malachi-您的说法有误。
\ $ \ endgroup \ $
–rolfl
2014年10月1日15:15

\ $ \ begingroup \ $
我无权运行Java代码,但是我可以使用C#创建一个随机数生成器,然后看看会发生什么...。
\ $ \ endgroup \ $
–马拉奇♦
2014年10月1日15:17

\ $ \ begingroup \ $
@Malachi,您将测试通过Java调用使用C#的RNG的效果。 Java的Random.nextInt(int n)返回0-(n-1),伪随机分布。
\ $ \ endgroup \ $
– Pimgd
2014年10月1日15:19



\ $ \ begingroup \ $
这里的元素分布非常不均匀。我已经测试了生成名称的500.000和5.000.000迭代,两次迭代都在前10个点给了我10个“匕首”项,第10个点的出现次数至少是第11位的两倍。因此,对于“匕首”有一定的偏爱。此外,在使用范围映射分布时,您将获得如下信息:gist.github.com/Vannevelj/3b57a23c2cd77523b641
\ $ \ endgroup \ $
– Jeroen Vannevel
2014年10月1日15:40

#4 楼

 rand = new Random(System.currentTimeMillis());


这不好。简单地使用new Random()会更好,因为即使每毫秒创建多个实例,它也可以确保不同的种子。从单个Random对象返回的序列彼此之间是随机的,不能保证从不同种子返回的序列不会以某种方式相关。如果在重新启动程序之前只生成几个数字,可能就是问题所在。


是,不是。虽然java.util.Random远非完美,但它具有48位状态,您可能会从几个样本中观察到相关性,恕我直言可以忽略。尝试SecureRandom看看您的程序是否存在缺陷。保证SecureRandom与完美的随机性源无法区分。

评论


\ $ \ begingroup \ $
我认为这不适用于这里。我认为很明显,该类是以不需要多个实例的方式编写的。
\ $ \ endgroup \ $
– Etheryte
2014年10月1日在16:28

\ $ \ begingroup \ $
@Nit True,但我们没有看到您的调用代码,因此我们不能保证。如果使用Random的no-arg构造函数,则改为使用System.nanoTime(),它的精度更高,这可以更好地保证随机种子是不同的。虽然最好的当然是总是重用该类。
\ $ \ endgroup \ $
–西蒙·福斯伯格
2014年10月1日在16:48



\ $ \ begingroup \ $
@Nit该类不是以单例形式编写的,因此我们不能说其实例数。当然,可能只有一个,所以没什么大不了的,但是较短且更好的构造函数无条件更好。
\ $ \ endgroup \ $
– maaartinus
2014年10月1日17:32

\ $ \ begingroup \ $
感谢您的输入。我已切换到较短的构造函数,并使该类成为单例。
\ $ \ endgroup \ $
– Etheryte
2014年10月1日20:20

\ $ \ begingroup \ $
@Nit请注意,除了这个,我不喜欢单身人士。但是,使用很少的单例并不坏,因为您可以通过DI轻松替换其单例。
\ $ \ endgroup \ $
– maaartinus
2014年10月1日20:52

#5 楼

虽然从单个Random对象返回的数字彼此之间是随机的,但不能保证从不同种子返回的序列不会以某种方式相关。如果在重新启动程序之前仅生成几个数字,可能就是问题所在。

以下代码演示了这种效果。在这里,我用10种不同的种子播种Random。输出如下。

import java.util.Random;

class Rander
{
    public static void main(String[] args)
    {
        for (long i = 0; i < 10; ++i)
        {
            Random r = new Random(i);
            System.out.println(r.nextInt());
        }
   }
}


这些代表将要产生的随机序列中的第一个数字。如您所见,它们之间并不是完全随机的!

播种随机数生成器确实可以使您的运行独立! >
-1155484576
-1155869325
-1154715079
-1155099828
-1157023572
-1157408321
-1156254074
-1156638823
-1158562568
-1158947317


现在您可以看到序列将不相关。

import java.security.SecureRandom;

class SecureRander
{
    public static void main(String[] args)
    {
        for (long i = 0; i < 10; ++i)
        {
            SecureRandom r = new SecureRandom();
            System.out.println(r.nextInt());
        }
    }
}


总之,从单个“随机”对象将足够满足您的目的,即使您使用播种,不同的运行相对于彼此也不是随机的。播种只会阻止它们完全相同。