8
\$\begingroup\$

I wrote a program that reads a text file and then outputs the unique words, their frequency, and the line numbers that word appears on. It also counts the number of sentences in the text file. All of this information is then outputted to a text file.

I currently have all of the code in one class object. What would be the correct way to divide up the functions / code so that I use OOP correctly? I have been reading up on separation of concerns but am having trouble applying it to this program. Should I make the code that handles input 1 separate class? Followed by a class for the "word tracking"? Then another class for output? Feel free to be as blunt as you like, I know it's not very good as is but I really want to improve it. I want to become better at implementing best practices of OOP in my programs.

class WordInfo
{ 
     private int sentenceTotal;
     LineNumberReader br = null;
     BufferedWriter bw = null;
     Map<String, ArrayList<Integer>> map = new TreeMap<String, ArrayList<Integer>>();

public WordInfo (int sens)
{
    sentenceTotal = sens; 
}
private void updateSenTotal(int curSens)
{
    sentenceTotal += curSens;
}
private void sentenceCount(String sen1)
{
    String senString = sen1;
    int sCounts = 0;
    String[] sCounter = senString.split("[.|?|!]");
    for (String counts : sCounter)
    {
        if(!counts.equals("  "))
        {
            sCounts++;
        }
    }
    sentenceTotal+=sCounts;
}
public void wordTrack()
{
    String currentLine = null;
    try 
    {
    while ((currentLine = br.readLine()) != null) 
    {
        if (!currentLine.isEmpty())
        {
            String senString = currentLine.toLowerCase();
            sentenceCount(currentLine);
            String[]parts = senString.split("\\W");
            for (String prints : parts)
            {
                // if the map does not contain the word, the word is added to the map and 
                // an array list of integers is also created for that key's value and
                // a value of 1 is put into the arraylist to count the frequency of the word
                // A line number value is also added to the arraylist at index 1.
                if (!map.containsKey(prints) && !prints.equals("") && prints.length() != 1)
                {
                    ArrayList<Integer> inter = map.get(prints);
                    inter = new ArrayList<Integer>();
                    map.put(prints, inter);
                    inter.add(1);
                    inter.add(br.getLineNumber());
                }
                // if the map already contains the word, then the frequency of the word is
                // incremented by 1 and the line number where the word was found is added to the list.  
                else if (map.containsKey(prints))
                {   
                    int adder = map.get(prints).get(0);
                    adder += 1;
                    map.get(prints).set(0, adder);
                    map.get(prints).add(br.getLineNumber());
                }
            }
        }
    }
}
    catch (IOException e) 
    {
        e.printStackTrace();
    }
}


public void fileIn(String fileName)
 {
      String name = fileName;
      try 
      {
        br = new LineNumberReader(new FileReader(name));
    }
    catch (IOException e) 
    {
        e.printStackTrace();
    }
} 
public void fileOut(String fileName)
{
    String name = fileName;
    try 
    {
        bw = new BufferedWriter(new FileWriter(name));
    }
    catch (IOException e) 
    {
        e.printStackTrace();
    }
} 
public void printFile()
{
try {
    for (String printKey : map.keySet())
    {
        bw.write(printKey + " | " + map.get(printKey).get(0) + " | ");

        for(int i = 1; i < map.get(printKey).size(); i++)
        {
            // if statement to ensure that when the final line number in the arraylist
            // is written to the file it is not followed by a comma
            if(i != map.get(printKey).size() - 1) 
            bw.write(map.get(printKey).get(i) + ", ");
            else
            bw.write(map.get(printKey).get(i) + "");
        }
        bw.write("\n");
    }
    bw.write("\nSentence count: " + sentenceTotal);
    bw.close();
    }
    catch (IOException e) 
    {
        e.printStackTrace();
    }
}

 public class OOPQuestion {

 public static void main(String[] args) throws IOException 
 {
    WordInfo info = new WordInfo(0);

    info.fileIn("/Users/Chris/Desktop/testingInput.txt");
    info.fileOut("/Users/Chris/Desktop/testOutput.txt");
    info.wordTrack();
    info.printFile();

    System.out.println("Count complete");
 }

 }
\$\endgroup\$
0
5
\$\begingroup\$

A couple of comments/questions:

  1. The LineNumberReader br, BufferedWriter bw, and Map<String, ArrayList<Integer>> map objects do not have the private keyword - do you want them to be private? If you want to practice object-oriented design, then you should.
  2. There are not enough comments in the code - it would be helpful if you have JavaDoc comments per method so that the reader knows exactly what you want to do for each method.
  3. Since you require unique words, I would use a Set object instead, instead of implementing the "checking if an object is there yourself" part.
  4. In your sentenceCounts method, what does the if(!counts.equals(" ")) code do? Did you mean to have multiple spaces in that String?
  5. Your wordTrack method has a catch (IOException e) but does not declare that it can throw an IOException in its header (the same can be said for the fileIn and fileOut methods).
  6. If you can help it (unless it is an exercise for you), do not use BufferedReader and the like to read from a file - either use a Scanner or some of the file classes in java.io.
  7. This may be just the formatting (after copy-pasting), but if you can indent the methods of the class by one tab, that would be great for the reader.
\$\endgroup\$
7
  • \$\begingroup\$ 1) I didn't know I could make those private to be honest. But yes I would want those to be private so I will adjust that. \$\endgroup\$ – Csw May 24 '15 at 18:58
  • \$\begingroup\$ 2) I'll edit in some more comments to further explain the code. \$\endgroup\$ – Csw May 24 '15 at 18:59
  • \$\begingroup\$ 3) So using a Set instead of a treemap would mean I can cut out the code where I check the map to see if it contains a word or not? 4) That if statement is there to eliminate the following from happening: 1) User types "The dog runs. " as the last sentence in a paragraph. He adds the 2 spaces on thinking he'd type more but doesn't. Notice the 2 spaces after the period If I split the strong according to punctuation then I get "The dog runs" and " ". The if statement ensures that the " " is not counted as a sentence. \$\endgroup\$ – Csw May 24 '15 at 19:05
  • \$\begingroup\$ @Corick 3) Yes, that is what Set is for (and if you needed a treemap, there is a TreeMap class). 4) If you can put that into the original description that can be helpful. \$\endgroup\$ – Ryan May 24 '15 at 19:07
  • 1
    \$\begingroup\$ @Corick I'd change the constructor so that you'd accept the String parameters and remove the fileIn and fileOut methods if necessary. \$\endgroup\$ – Ryan May 24 '15 at 22:21
3
\$\begingroup\$

I would organize (split the program in classes/methods) this application in the following way (in pseudo-code):

class WordInfo {
  int count;
  List<Integer> lineNumbers;
}

class FileInfo {
  Set<String, WordInfo>
  int sentenseCount

  void dump(Writer writer) {
      ... (code from printFile) ...
  }

  static FileInfo collect(Reader reader) {
     ... (code from wordTrac) ....
  }
}

void main(String[] args) {
  try (
     Reader reader = new FileReader(args[0]),
     Writer writer = new FileWriter(args[1])
  ) {
    FileInfo fi = FileInfo.collect(reader)
    fi.dump(writer)
  }
}
\$\endgroup\$
1
1
\$\begingroup\$

Both previous answers are helpful. In addition I wouldn't use

bw.write("\n");

as its not working for me. Instead you can use the specific BufferedWriter method for this:

bw.newLine();

A newLine() method is provided, which uses the platform's own notion of line separator as defined by the system property line.separator. Not all platforms use the newline character ('\n') to terminate lines. Calling this method to terminate each output line is therefore preferred to writing a newline character directly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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