Very basic Context Free Grammar in Java

I wrote this program for an assignment, thus I had some requirements as far as the format of the cfg.txt file and some basic other classes that we had to use. Other than that, I am curious if there is a way to drastically simplify this code. Although the program runs fine and produces the desired output, is there a better way to go about writing this? (What initially spurred my concern was the iterateData method. I did not like the code repetition and thought it may be convenient to "reuse" some of it. However, in doing so, I then introduced numerous class variables to prevent from having to pass in 7 instance variables to the method. I can't say that I'm all too thrilled about this...) Regardless, advice and suggestions appreciated.

Thank you.

```
import java.util.Scanner;
import java.io.*;
import java.util.Random;

public class ContextFreeGrammar {

private static String nt, useExp = "";
private static int totalWeight = 0, randomNum;
private static ArrayUnsortedList data = new ArrayUnsortedList();
private static Production pObj;

public static void main(String[] args) throws IOException {

String result = "";
File myFile = new File("cfg.txt");
Scanner fileScan = new Scanner(myFile);

// Read and process each line of the file
while (fileScan.hasNext()) {
Production p = new Production(fileScan.nextLine());
data.add(p);
}

// If the result string still contains non-terminals
while (result.contains("'));

// Cannot condense because we have an ambiguous
// totalWeight. Hence, we need to iterate twice:
// 1. To generate appropriate random number from totalWeight
// 2. To determine when the totalWeight exceeds that random number.

iterateData(false);

Random rand = new Random(); // Generate a random number object
randomNum = rand.nextInt(totalWeight - 1) + 1; // Generate actu

-
It is said that "a method should do one thing, and it should do that one thing well". Your iterateData method is being used to do two separate things, depending on the parameter you send it.

-
If your ArrayUnsortedList is this class (which really could use a code review itself) then I don't really understand why you are using it. The way I see it is that you could use a regular ArrayList.

-
What you are referring to as "class variables" are static variables. It would be better to use them as instance variables. Currently you can't have two ContextFreeGrammar objects at once, each with it's own state. Remove the static keyword and *create an instance of ContextFreeGrammar to properly use it as an instance (which is a lot better practice, although you might not have learned about such things yet. If you haven't learned about it yet, you should ask your teacher about it)

-
Only create a new Random object once. Random objects are meant to be re-used. When they are initialized, they get a random seed set depending on the system clock time. Repeatedly creating a Random object gives a loss in randomness.

-
Instead of your current iterateData method, consider using one method such as int calculateTotalWeight(List list, String nonTerminal) and one String determineExpression(List list, String nonTerminal, int weightRandom). Determining such an important behavior of your method by sending it a true/false value makes your code harder to understand. You don't need to pass seven variables to the method when it only uses two or three.

StackExchange Code Review Q#37058, answer score: 4