【JAVA】文字列をシャッフルするコードで11文字以上になると動かない

Question

前提・実現したいこと

入力された文字列の順番をバラバラにして出力するコードを書いてみたのですが、
文字数が11文字以上だとうまく動かず、
原因が分からなかったため投稿しました。
よろしくお願いします。

該当のソースコード

import java.util.Random;
import java.util.Scanner;

public class Main{
	public static void main(String[] args) {
		StringBuilder sb = new StringBuilder();
		Scanner sc = new Scanner(System.in);
		Random rn = new Random();

		String S = sc.next();
		char[] sch = S.toCharArray();
		Integer slen = sch.length;
		char[] ans = new char[slen];

		for(int i=0;i<slen;i++) {

			for(int j=0;j<1;) {
			Integer r = rn.nextInt(slen);
			String a = sb.toString();
			if(a.contains(r.toString())) {
			}else {
				ans[i]=sch[r];
				j++;
				sb.append(r).append(".");
			}
			}
		}
		for(int i=0;i<ans.length;i++) {
			System.out.print(ans[i]);
		}
		sc.close();
	}
}

Answer 1

質問で書いているコードは、「まだ出てない番号が出るまで乱数を回し続ける」という方法になるかと思いますが、この方法では、あとになればなるほど「まだ出てない番号」が出る確率が低くなっていき、終わるまでに時間がかかる恐れがあります。
「シャッフルする」のであれば、次のような方法が一般的です。
「抽選箱に文字をすべて入れて、取り出した順に後ろから並べていく」というような方法です。

import java.util.Random;
import java.util.Scanner;

public class Main{
    public static void main(String[] args) {
        try (Scanner sc = new Scanner(System.in)) {
            Random rn = new Random();
            String S = sc.next();
            char[] sch = S.toCharArray();
            for (int i = S.length() - 1; i > 0; i--) {
                int x = rn.nextInt(i + 1);
                if (x == i) continue;
                char temp = sch[x];
                sch[x] = sch[i];
                sch[i] = temp;
            }
            System.out.println(sch);
        }
    }
}

Comments

[nest]

回答をありがとうございます！
教えていただいたコードも分かりやすくて理解することができました。
取り出す数字の候補を減らしていくところがすごい面白いですね。
精進します！

Answer 2

中身は理解できていません。ただ、if (a.contains(r.toString()))がtrueになったとき、jはインクリメントされない。これを繰り返して、「文字数が11文字以上だとうまく動かず」となっているのではないかと。

たとえは、sbが3.4.10.でrが1だと、件のif文はtrueになりますから。

      for (int j = 0; j < 1; ) {
        Integer r = rn.nextInt(slen);
        String a = sb.toString();
        if (a.contains(r.toString())) {
        } else {
          ans[i] = sch[r];
          j++;
          sb.append(r).append(".");
        }
      }

StringBuilderではなく、List<Integer>とかで済チェックすればいいとおもいますけど。

Comments

[nest]

解答をありがとうございます！
仰る通りですね。。
この方法だとだめな原因がとてもよく分かりました・・。
教えていただいたListはまだ触れたことがなかったので勉強してみます！

[shiketa]

勉強のために作ってみるのはいいことです。
が、ナニかに組み込むのであれば、すでにある車輪を使ったほうがよろしいかと。Collections#shuffle()
https://docs.oracle.com/javase/jp/8/docs/api/java/util/Collections.html#shuffle-java.util.List-

[nest]

ありがとうございます！
そうですね、実際に使うとなったら用意されてるものを活用すべきですよね。
まだJAVAの知識が無さすぎるので、貼っていただいたリンク先のコレクション群もいつか全部理解できるように、頑張ります！