Tetris random piece generator

Question

I was inspired by this Reddit /r/dailyprogrammer challenge:

Contrary to popular belief, the tetromino pieces you are given in a game of Tetris are not randomly selected. Instead, all seven pieces are placed into a "bag." A piece is randomly removed from the bag and presented to the player until the bag is empty. When the bag is empty, it is refilled and the process is repeated for any additional pieces that are needed.

The pieces are as follows: O, I, S, Z, L, J, T

I was just wanting some general feedback on the code regarding readability, structure, and efficiency.

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Scanner;

/**
 * Created on 6/23/2016.
 *
 * Inspired by r/dailyprogrammer
 * https://www.reddit.com/r/dailyprogrammer/comments/3ofsyb/20151012_challenge_236_easy_random_bag_system/
 */


public class RandomBag {

    /**
     * This is the main method. The main acquires input from the user
     * then generates a combination of pieces in a particular order.
     * @param args Unused.
     */
    public static void main(String args[]){
        Scanner scanner = new Scanner(System.in); // Used to obtain user input.
        loop : while (true) {
            System.out.print(">>> ");
            String input = scanner.nextLine().replace(" ", "");

            // Checks for special cases.
            switch (input) {
                case "exit" : break loop;
                case "" : continue loop;
            }

            // The generator class will return a string of its results.
            System.out.println(new Generator(Integer.parseInt(input)));
        }
    }

    /**
     * Created on 6/23/2016.
     * Used to create a bag that generates a set amount of pieces in a combination
     * that will never have the same piece displayed 3 times in a row. This is
     * accomplished by placing all pieces in a list and removing them one by one.
     * Once the list has been depleted the list is renewed with the same pieces.
     */


    private static class Generator{
        private int pieceNumber; // Number of pieces that will be generated.
        private String generation = ""; // Used to store the piece generation.
        private final String[] pieces = {"O", "I", "J", "L", "S", "Z", "T"}; // Pieces used in the generation.

        /**
         * Constructor used to store the number of pieces that need to be
         * generated and execute the piece generation method.
         * @param pieceNumber Used to determine how long the generation should be.
         */
        Generator(int pieceNumber){
            this.pieceNumber = pieceNumber; // Initializes number of pieces.
            generatePieces(); // Generates piece combination.
        }

        /**
         * Used to generate the pieces in the specific order. Manipulates class instanced variables.
         */
        private void generatePieces(){
            ArrayList<String> temp = new ArrayList<>(Arrays.asList(pieces)); // Used to store all pieces.
            while(generation.length() <= pieceNumber){ // Generates string until it is the specified size.
                if(temp.isEmpty()) // Checks if list is empty.
                    temp = new ArrayList<>(Arrays.asList(pieces)); // Replaces the pieces in the list.
                Collections.shuffle(temp); // Shuffles the list
                generation += temp.get(0); // Places the first element of the list onto the string.
                temp.remove(0); // Removes the element that was added from the list.
            }
        }

        /**
         * Returns the generation constructed by the generatePieces method.
         * @return The string generated.
         */
        @Override
        public String toString(){
            return generation; // Returns piece generation once generated.
        }
    }
}

Answer 1

The hard part of the challenge is already done for you by Collections.shuffle(). I'll focus on style, efficiency, and elegance.

In the main() function, scanner.nextLine().replace(" ", "") is a weird thing to do. Would it be acceptable to interpret 5 0 as 50? Perhaps you meant to do String.trim()?

Style

You have written a comment on every single line, which is too much. Only beginner programmers are likely to want to state everything twice. This comment, for example, is silly:

Collections.shuffle(temp); // Shuffles the list

The two lines above it aren't good either. The // Checks if list is empty comment is noise. You should not omit the "optional" braces. You should pick a better variable name than temp.

if(temp.isEmpty()) // Checks if list is empty.
    temp = new ArrayList<>(Arrays.asList(pieces)); // Replaces the pieces in the list.

Efficiency

Repeated concatenation of strings using += is not recommended. Concatenating n characters that way is O(n²), since each result has to be rebuilt from scratch. Instead, use a StringBuilder (if you don't know the length of the final result) or a char[] (if you know exactly how long the final string will be).

Calling .remove(0) on an ArrayList is also not recommended. To fill in the gap after removal, it has to copy over all of the subsequent elements. Emptying an n-element ArrayList by repeatedly calling .remove(0) is O(n²). Instead of removing the first element, you could remove the last element. Better yet, just keep track of the index yourself, so that you don't have to remove anything, and you don't have to keep re-creating the list.

Elegance

A more elegant solution, I think, would be to generate an infinite stream, then take an excerpt from it of the desired length. Such an infinite stream could actually be used as part of a real Tetris implementation. Your Generator, on the other hand, has to be instantiated with a length limit (awkwardly named pieceNumber).

Suggested solution

This solution, using streams, produces an infinite sequence, then limits it to length 50.

You can easily tweak it to work like your interactive loop.

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class ShuffledStream {
    private ShuffledStream() {}   // Suppress the default constructor

    /**
     * Produces an infinite stream of the elements, reshuffling when
     * each batch is exhausted.
     */
    public static <T> Stream<T> stream(T[] elements) {
        return Stream.iterate(Arrays.asList(elements), list -> {
            Collections.shuffle(list = new ArrayList<T>(list));
            return list;
        }).skip(1)              // First batch is unshuffled; skip it
          .flatMap(List::stream);
    }

    public static void main(String[] args) {
        String fifty = ShuffledStream.stream("OISZLJT".split(""))
                                     .limit(50)
                                     .collect(Collectors.joining());
        System.out.println(fifty);
    }
}

Note that since this solution uses generics, you can reuse the same code to deal from 52-card decks.