Code Optimization or changing code

[Jason Ram]

Hi I have the following code that I have written but I want to optimize it if possible, or change it up a little. A friend helped me with some of the concepts behind it and I would like to see other ways it an be implemented as well. Thanks
Here is the code with the doc strings left in

/*
TextProcessorImpl.java
 */

import java.util.*;

/**
 * TextProcessorImpl uses the TextProcessor interface to:
 * Find largest word in a file
 * Find the shortest word in a file
 * Find the most common first letter in all words in the document
 * Find the most common word of a given length in the file
 * Find what word of a given length has the highest number of english vowels in the file
 * @author [removed for privacy]
 */
public class TextProcessorImpl implements TextProcessor {

    /**
     * The hashmap that the constructor populates from the file with the
     * words as keys and their occurrences as values.
     */
    private HashMap<String, Integer> hash;


    /**
     * Constructor that takes one parameter, a file, and constructs a Hashmap
     * of words and their occurrences to be used by the methods in this class.
     * Words containing the digits 0-9 are ignored, and any punctuation in a word is stripped before storage.
     * Words that were purely punctuation are ignored.
     * @param file The file that is going to be read
     */
    public TextProcessorImpl(String file) {
        TextReader read = new TextReader();
        Scanner scan = read.openFile(file);
        hash = new HashMap<>();
        while (scan.hasNext()) {
            String entry = scan.next();
            if (!(entry.matches(".*\\d+.*"))) {
                String word = entry.replaceAll("\\p{P}", "");
                if (hash.containsKey(word)) {
                    int count = hash.get(word);
                    hash.put(word, count + 1);
                }
                if (!(hash.containsKey(word))) {
                    hash.put(word, 1);
                }
            }
        }
    }

    /**
     * Return the letters that are most likely to appear at
     * the start of a word from the literary work. Frequency
     * of the words involved is not taken into account. There
     * will often only be one letter, but in case of a tie,
     * a collection is returned.
     * @return the most common first letter(s) in the words
     */
    public Collection<Character> mostCommonFirstUnweighted() {
        HashMap<Character, Integer> map = new HashMap<>();
        Collection<Character> letters = new ArrayList<>();
        for (String word : hash.keySet()) {
            if (!(word.isEmpty())) {
                if (map.containsKey(word.charAt(0))) {
                    int count = map.get(word.charAt(0));
                    map.put(word.charAt(0), count + 1);
                }
                if (!(map.containsKey(word.charAt(0)))) {
                    map.put(word.charAt(0), 1);
                }
            }
        }
        int max = 0;
        for (Character letter : map.keySet()) {
            if (map.get(letter) == max) {
                letters.add(letter);
            }
            if (map.get(letter) > max) {
                letters.clear();
                letters.add(letter);
                max = map.get(letter);
            }


        }
        return letters;
    }


    /**
     * Return the longest word or words from the literary work.
     * There will often only be one word, but in case of a tie,
     * a collection is returned.
     * @return the longest word(s)
     */
    public Collection<String> getLongestWords() {
        Collection<String> longest = new ArrayList<>();
        int max = 0;
        for (String word : hash.keySet()) {
            if (word.length() == max) {
                longest.add(word);
            }
            if (word.length() > max) {
                longest.clear();
                longest.add(word);
                max = word.length();
            }

        }
        return longest;
    }


    /**
     * Return the shortest word or words from the literary work.
     * Since most languages contain many 1-letter words,
     * a collection is returned.
     * @return the shortest word(s)
     */
    public Collection<String> getShortestWords() {
        Collection<String> shortest = new ArrayList<>();
        int min = 1000;
        for (String word : hash.keySet()) {
            if (!(word.isEmpty())) {
                if (word.length() == min) {
                    shortest.add(word);
                }
                if (word.length() < min) {
                    shortest.clear();
                    shortest.add(word);
                    min = word.length();
                }
            }
        }
        return shortest;
    }


    /**
     * Return the most common word or words of a given
     * length in the literary work. There will often
     * only be one such word, but in case of a tie,
     * a collection is returned.   If no words exist of
     * the desired length, an empty collection is returned.
     * @param length the length of words to consider
     * @return the collection of most common words
     */
    public Collection<String> mostCommon(int length) {
        Collection<String> common = new ArrayList<>();
        int occurrence = 0;
        for (String word : hash.keySet()) {
            if (word.length() == length) {
                if (hash.get(word) == occurrence) {
                    common.add(word);
                }
                if (hash.get(word) > occurrence) {
                    common.clear();
                    common.add(word);
                    occurrence = hash.get(word);
                }

            }
        }
        return common;
    }


    /**
     * Return the word or words with the most vowels of a given
     * length in the literary work.  Here, we consider only
     * the letters A, E, I, O and U as vowels (upper or lowercase).
     * If no words exist of the desired length, an empty collection is returned.
     * @param length the length of words to consider
     * @return the collection of the words of given length with the most vowels
     */
    public Collection<String> mostVowelsWordLength(int length) {
        Collection<String> mostVowels = new ArrayList<>();
        int vowelcount = 1;
        for (String word : hash.keySet()) {
            if (word.length() == length) {
                int vowels=0;
                for (int ch=0; ch < word.length(); ch++) {
                    if(word.substring(ch,ch+1).toLowerCase().matches("[aeiouAEIOU]")){
                            vowels++;
                        }

                }
                if (vowels == vowelcount) {
                    mostVowels.add(word);
                }
                if (vowels > vowelcount) {
                    mostVowels.clear();
                    mostVowels.add(word);
                    vowelcount = vowels;
                }

            }
        }
        return mostVowels;
    }


    /**
     * Return the letters that are most likely to appear at
     * the start of a word from the literary work. Frequency
     * of the words involved is taken into account. There
     * will often only be one letter, but in case of a tie,
     * a collection is returned.
     * @return the most common first letter(s) in the words
     */
    public Collection<Character> mostCommonFirstWeighted() {
        Collection<Character> mostCommon = new ArrayList<>();
        HashMap<Character, Integer> map = new HashMap<>();
        int count = 0;
        for (String word : hash.keySet()) {
            if (!(word.isEmpty())) {
                if (map.containsKey(word.charAt(0))) {
                    int num = map.get(word.charAt(0));
                    map.put(word.charAt(0), num + hash.get(word));
                }
                if (!(map.containsKey(word.charAt(0)))) {
                    map.put(word.charAt(0), hash.get(word));
                }
            }
        }
        int max = 0;
        for (Character letter : map.keySet()) {
            if (map.get(letter) == max) {
                mostCommon.add(letter);
            }
            if (map.get(letter) > max) {
                mostCommon.clear();
                mostCommon.add(letter);
                max = map.get(letter);
            }

        }
        return mostCommon;
    }
}

[Les Morgan]

Well, Here are my thoughts:

Java has the JIT compiler and it is a fairly smart compiler, you can do all of the tricks in the world, but you are not guaranteed that it will have any impact because Java is going to optimize things behind the scenes for you and in spite of you. So the question is: do you have any bottlenecks that show in your system, are there segments that you see unexpected delays?

My best words on optimization in Java comes down to this:

Write dumb code, code that is textbook example and quality--don't try to be tricky--the compiler will usually do a much better job at optimization than you can.

If there is obvious delays in segments of your code, then work on optimization, run performance tests and etc in those sections, but unless you are seeing a problem--you are usually chasing your tail because Java is optimizing behind the scenes and your efforts may have little to no impact.

Les

[Winston Gutkowski]

J Awesome wrote:Hi I have the following code that I have written but I want to optimize it if possible, or change it up a little.

Well, the very first question then is: Does it work? Every time and for any input? Because there's absolutely no point in changing or optimizing anything until it does.

Second question: What does "change it up a little" mean? Phrases like that are vague, and suggest that you don't have a proper goal in mind - and that's a really bad way of working. So I'd definitely define exactly what you mean by "change it up" before you alter a single line of code.

A friend helped me with some of the concepts behind it and I would like to see other ways it an be implemented as well.

OK, well there are a couple of things missing:
1. Your class extends TextProcessor, but we have no idea what it does.
2. It uses a TextReader, but we have no idea what that does either.
3. Your program appears to do an awful lot of things with "words", yet you don't have a Word class.
and also:
4. You appear to put words straight into your HashSet, which means that "hello" and "Hello" will be treated as different words. Is that what you want?


Now to "efficiency":
1. You might find that a HashMap<String, AtomicInteger> is a bit faster, since you can increment AtomicIntegers without having to create a new object every time.
2. You appear to populate all your Collections very specifically, including separate ones for longest and shortest.
If you used a List instead of a Collection, and had a Comparator that compares by word length, you could simply plough hash.keyset() straight into it and then sort it by length. Then the shortest words will be at the start and the longest ones at the end.
It may actually take longer than what you're doing, but I suspect you'll then have a more useful collection for doing other things.


Finally, going back to a Word class, here's a rudimentary one that might give you some ideas:

public final class Word
   implements Comparable<Word>
{
   private final String word;
   private final int vowelCount;
   private final int hash = 0;

   // Private c'tor; used by factory method below.
   private Word(String s) {
      word = s.trim();
      String lc = word.toLowerCase();
      vowelCount =
         lc.length() - lc.replaceAll("[aeiou]", "").length();
      hash = lc.hashCode();
   }

   // Factory method
   public static final Word from(String s) {
      return isWord(s) ? new Word(s) : null;
   }

   public static final boolean isWord(String s) {
      for (char c : s.trim().toCharArray()) {
         if (!Character.isLetter(c))
            return false;
      }
      return true;
   }

   public final int getLength() {
      return word.length();
   }

   public final int getVowelCount() {
      return vowelCount;
   }

   @Override
   public final int compareTo(Word w) {
      return word.compareToIgnoreCase(w.word); 
   }

   @Override
   public final boolean equals(Object obj) {
      return obj == this ||
        (obj instanceof Word && compareTo((Word) obj) == 0);
   }

   @Override
   public final int hashCode() {
      return hash;
   }

   @Override
   public final String toString() {
      return word;
   }
  
   ...
}

Do you see the idea? You now have a class that:
(a) Compares with other "words" lexically.
(b) Has an equals() method that is consistent with both compareTo() and hashCode().
(c) Returns its length and a vowel count.
which are all things that you seem to need.

My version treats "Hello" and "hello" as the same word, but you don't have to. The idea is to create a class that does what YOU want a "word" to do.

HIH

Winston

[Knute Snortum]

if (!(entry.matches(".*\\d+.*"))) {

Two things about line 38. When you have a regex inside a loop, it is always better to "pre-compile" the regex.

public class TextProcessorImpl implements TextProcessor {
    private static final String GET_DIGITS = ".*\\d+.*";
    // code omitted
    public TextProcessorImpl(String file) {
        Pattern pattern = Pattern.compile(GET_DIGITS);
        // code omitted
        if (!pattern.matcher(entry).matches()) {

The second thing is that since you are using a matcher, you might as well use find(). Whenever you see yourself writing a pattern like .*anything.*, you should probably use find().

public class TextProcessorImpl implements TextProcessor {
    private static final String GET_DIGITS = "\\d+";
    // code omitted
    public TextProcessorImpl(String file) {
        Pattern pattern = Pattern.compile(GET_DIGITS);
        // code omitted
        if (!pattern.matcher(entry).find()) {

[Campbell Ritchie]

Your version also avoids writing return twice in the same method

[Jason Ram]

Hi, I will post the other 3 classes that I am using in my project when I get back to my desktop in an hour or two. thank you for the input so far.

[Jason Ram]

Here are the other 3 classes I have that were supplied to me.\

import java.util.Collection;


/**
 * Main program
 * @author [removed for privacy]
 * @author [removed for privacy]
 */



public class TextAnalysis {


    /**
     * Get a file name from the command line and ask a TextProcessor
     * to analyze it.
     *
     * @param args a single-element array containing the file name
     */
    public static void main( String[] args ) {
        if ( args.length != 1 ) {
            System.err.println( "Usage: java TextAnalysis file" );
            System.exit( 2 );
        }
        TextProcessor textProc = new TextProcessorImpl( args[ 0 ] );

        Collection< String > longestWords = textProc.getLongestWords();
        System.out.println( "Longest words: " + longestWords );

        Collection< String > shortestWords = textProc.getShortestWords();
        System.out.println( "Shortest words: " + shortestWords );

        System.out.println(
                "Most common words of length 5: " +
                        textProc.mostCommon( 5 )
        );

        System.out.println(
                "Most common first character: " +
                        textProc.mostCommonFirstWeighted()
        );

        System.out.println(
                "Most common first character, ignoring word frequency: " +
                        textProc.mostCommonFirstUnweighted()
        );

        System.out.println(
                "Words of length 10 with the most vowels : " +
                        textProc.mostVowelsWordLength( 10 )
        );
    }
}

/*
TextReader.java
 */


import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.InputStream;
import java.util.Scanner;


/**
 * Convenience class for programmers who don't know file
 * input/output in Java. ... yet
 */


public class TextReader {


    /**
     * Produce a Scanner instance connected to the given file.
     * If the file cannot be opened, this method will report
     * it on standard error, and then terminate the program
     * with a status of 1.
     * @param fileName the name of the file to be scanned
     * @return a Scanner instance on the file. (Please close it when done!)
     */
    public static Scanner openFile( String fileName ) {
        Scanner result = null;
        try {
            InputStream istream = new FileInputStream( fileName );
            result = new Scanner( istream );
        }
        catch ( FileNotFoundException fnfe ) {
            System.err.println( fnfe );
            System.err.println( "File \"" + fileName + "\" does not exist." );
            System.exit( 1 );
        }
        return result;
    }
}

import java.util.Collection;



/**
 * Interface for classes that read in a literary work and analyze word use.
 *
 * (For specifications of the constructor for those classes, see the
 * laboratory assignment writeup.)
 *
 * @author [removed for privacy]
 * @author [removed for privacy]
 */


public interface TextProcessor {


    /**
     * Return the letters that are most likely to appear at
     * the start of a word from the literary work. Frequency
     * of the words involved is not taken into account. There
     * will often only be one letter, but in case of a tie,
     * a collection is returned.
     * @return the most common first letter(s) in the words
     */
    Collection<Character> mostCommonFirstUnweighted();


    /**
     * Return the longest word or words from the literary work.
     * There will often only be one word, but in case of a tie,
     * a collection is returned.
     * @return the longest word(s)
     */
    Collection< String > getLongestWords();


    /**
     * Return the shortest word or words from the literary work.
     * Since most languages contain many 1-letter words,
     * a collection is returned.
     * @return the shortest word(s)
     */
    Collection< String > getShortestWords();


    /**
     * Return the most common word or words of a given
     * length in the literary work. There will often
     * only be one such word, but in case of a tie,
     * a collection is returned.   If no words exist of
     * the desired length, an empty collection is returned.
     * @param length the length of words to consider
     * @return the collection of most common words
     */
    Collection< String > mostCommon(int length);


    /**
     * Return the word or words with the most vowels of a given
     * length in the literary work.  Here, we consider only
     * the letters A, E, I, O and U as vowels (upper or lowercase).
     * If no words exist of the desired length, an empty collection is returned.
     * @param length the length of words to consider
     * @return the collection of the words of given length with the most vowels
     */
    Collection< String > mostVowelsWordLength(int length);


    /**
     * Return the letters that are most likely to appear at
     * the start of a word from the literary work. Frequency
     * of the words involved is taken into account. There
     * will often only be one letter, but in case of a tie,
     * a collection is returned.
     * @return the most common first letter(s) in the words
     */
    Collection< Character > mostCommonFirstWeighted();
}