0
\$\begingroup\$

I have written a probability table generator in Java to help me generate randomized girl's names for the game I am making.

It reads from a text file and creates a HashMap<String, Double> full of two-character combinations and their rate of incidence.

I don't have a lot of experience with either probability tables or Java in general.

I would like help making it run faster

Example text from input file:

Each line has ten names. The real text file has 462 lines, and 4620 names.

Marva Kandy Debbi Louisa Edris Orpha Lynn Louise Reyna Jacki Evangeline

Ray Sienna Alecia Elin Debby Noreen Alishia Michael Evangelina Eliz

Usha Ladawn Dominga Cynthia Marty Shayla Sanora Betsey Ira Jodee

Qiana Elia Elke Torie Amirah Marta Lavonna Ozie Lavonne Teofila

Latoria Catharine Epifania Susy Keli Delpha Isa Marth Rosina Marti

Jordan Ella Theda Tyesha Rana Francesca Shayna Letha Shayne Marry

Rea Joellen Fidela Kandi Celeste Salina Kamryn Jenelle Celena Celesta

Kenyatta Ada Shelia Maribel Edith Lorilee Jazmyn Myrtice Kena Laurence

My code

public class MCVE {

    public static void main(String[] args) throws Exception {
        // create the file-path
        String address = "./res/text_files/Human_FirstNames";
        // will hold letter combinations and their frequency
        HashMap<String, Double> frequencies = new HashMap<>();

        FileReader fr = new FileReader(address);
        BufferedReader br = new BufferedReader(fr);

        // Iterate through file
        // get rid of new-line characters and concat to wholeString
        String aLine = "";
        String wholeString = "";
        while((aLine = br.readLine()) != null) {
            wholeString += aLine.replaceAll(System.lineSeparator(), " ");
        }

        // convert names to lowercase and into char[]
        char[] charArr = wholeString.toLowerCase().toCharArray();

        // iterate through char[] charArr and
        // add the number of times each character-combination is shown
        // to the value for each character-combination key.
        int length = charArr.length;
        String tempString = "";
        for(int i = 0; i < length; i++) {

            if(i + 1 == length)
                break;

            if(charArr[i] != ' ' && charArr[i + 1] != ' ') {
                tempString = Character.toString(charArr[i]) + Character.toString(charArr[i + 1]);
                if (frequencies.containsKey(tempString))
                    frequencies.replace(tempString, frequencies.get(tempString) + 1.0);
                else // if it doesn't contain this key yet, create it
                    frequencies.put(tempString, 1.0);
            }
        }

        // iterate through HashMap frequencies and
        // replace the values with the product of
        // (original_value / size_of_HashMap)
        double mapLength = frequencies.size();
        Iterator iter = frequencies.entrySet().iterator();
        while(iter.hasNext()) {
            HashMap.Entry pair = (HashMap.Entry)iter.next();

            frequencies.replace((String) pair.getKey(), (Double) pair.getValue() / mapLength);

            System.out.println(pair.getKey() + " " + pair.getValue());
            iter.remove();
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Your latest edits can be tolerated, but please don't make any further changes. \$\endgroup\$ – Mast Dec 25 '18 at 21:50
  • \$\begingroup\$ @Mast Thanks for the heads-up. I was already editing the file when the answer was posted. It has not been edited to accommodate it. \$\endgroup\$ – LuminousNutria Dec 25 '18 at 21:53
1
\$\begingroup\$

I have looked through your code. Here are a few things I notice

  1. StringBuilder instead of using += on String, for more efficient string concatenation.

Creating a new instance of StringBuilder and using that would work better than using the + operator on Strings. Something like this:

StringBuilder builder = new StringBuilder(); // inside the loop call the following builder.append("some string to append"); // ... // later on when you need the string builder.toString();

  1. More descriptive variable names

Try to explain what your variable represents instead of just calling it wholeString. To you, you may very well remember the significance of each variable and what it represents, but this is a good coding practice to get into because code will be read more than written and it would be easier for anyone else reading your code to immediately tell what it's doing without having to in many cases even read comments. This is called "self-documenting code".

  1. JDK 8+ Files API

After JDK 8 you can call lines() using the Files API. Although your approach is correct using BufferedReader and FileReader, you may want to consider switching to using the lines() method and operating on the Stream returned because it is a bit less verbose which makes reading it easier.

  1. Succinct method chaining using Stream over iterative approach

If you switch to using Stream for the file, you can easily adopt a more functional approach of calling filter or other stream related functions on it. This is a bit cleaner to read than the iternative while loop to remove the newlines in my view.

  1. Multiline comments using multiline comment syntax

For in-depth explanations and long comments that span to multiple lines, you can use the following notation:

/*

This comment

spans

multiple lines

*/

However, with that being said - I think the comments in this code are a bit much. Using self documenting variables as mentioned above, comments can be lessened and the code would be more readable too.

Hope that helps, let me know if you have any questions on the feedback I've given.

\$\endgroup\$

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.