I am a semi-new Java programmer that tends to be a perfectionist. What I'd like to know about my code:
- How it compares to common practice.
- Is it readable?
- Are my comments appropriate?
- Any improvements that could be made?
I'm not asking to bog down your time to do an in depth review, just off a glance. I received a poor score from a professor with no explanation other than it "Not being suitable", though it follows all directions posted.
My program:
This program takes a word from a file specified by a user and puts it in a location specified by a user. It takes a single word from the file at a time and places it in a blocking queue. It then goes to a different thread that will reverse every other word starting with the second word (not including punctuation). This thread places it into a different blocking queue for the last thread to write to the file specified.
Main:
import java.io.File;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;
import javax.swing.JFileChooser;
/**
* Test the producer consumer package
*
* 25/10/2014
*
* @author Tyler Weaver
*/
public class Tester {
public static void main(String[] args) {
final int MAX_SIZE = 2; //Max queue size
//Declarations
File input, output;
JFileChooser chooser = new JFileChooser();
BlockingQueue<CharSequence> fromReader
= new ArrayBlockingQueue(MAX_SIZE);
BlockingQueue<CharSequence> toWriter
= new ArrayBlockingQueue(MAX_SIZE);
ExecutorService service = Executors.newCachedThreadPool();
int returnVal, exitVal;
//Declarations
//Retrieves the input file
do {
input = null;
returnVal = chooser.showOpenDialog(null);
if (returnVal == JFileChooser.APPROVE_OPTION) {
input = chooser.getSelectedFile();
}
} while (returnVal != JFileChooser.APPROVE_OPTION);
//Retrieves the input
//Retrieves the output file
do {
output = null;
exitVal = chooser.showSaveDialog(null);
if (exitVal == JFileChooser.APPROVE_OPTION) {
output = chooser.getSelectedFile();
}
} while (exitVal != JFileChooser.APPROVE_OPTION);
//Retrieves the output file
//Declare Runnable objects
Runnable reader = new WordReader(input, fromReader);
Runnable rev = new WordReverser(fromReader, toWriter);
Runnable writer = new WordWriter(output, toWriter);
//Execute threads and shutdown
service.execute(reader);
service.execute(rev);
service.execute(writer);
service.shutdown();
}
}
WordReader: The thread that reads the words from a file one at a time.
package ProducerConsumerAssignment;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.util.concurrent.BlockingQueue;
import java.util.Scanner;
/**
* Reads words from a file and places them into a blocking queue to be read
*
* 25/10/2014
*
* @author Tyler Weaver
*/
public class WordReader implements Runnable {
//The blocking queue to store
private static BlockingQueue<CharSequence> bin;
private final File loc; //File to read from
private final String END_FLAG = "Terminate the queue";
//^Pill for the Poison Pill Technique. Works because every String sent
//through the queue will always be one word. Because this is multiple words
//There is no way another string could have the same value.
/**
* Constructor for WordReader
*
* @param input the text file to read from
* @param bin the blocking queue to store the words
*/
public WordReader(final File input, BlockingQueue bin) {
loc = input;
WordReader.bin = bin;
}
/**
* Called when being executed Reads words from a file and places into a
* blocking queue
*/
@Override
public void run() {
try (Scanner in = new Scanner(new FileReader(loc))) {
while (in.hasNext()) {
bin.put(in.next());
}
//Once no more words in the file send the poison
bin.put(END_FLAG);
} catch (FileNotFoundException ex) {
System.err.printf("Error finding File!%n%s%n", ex);
} catch (InterruptedException ex) {
System.err.printf("File Reader thread was interrupted!%n%s%n", ex);
}
}
}
Word Reverser: The thread that reverses every other word starting with the second word.
package ProducerConsumerAssignment;
import java.util.concurrent.BlockingQueue;
/**
* Takes a word from a blocking queue and reverses it. Puts the reversed word
* into another blocking queue.
*
* 25/10/2014
*
* @author Tyler Weaver
*/
public class WordReverser implements Runnable {
private final String END_FLAG = "Terminate the queue";
//^Pill for the Poison Pill Technique. Works because every String sent
//through the queue will always be one word. Because this is multiple words
//There is no way another string could have the same value.
private static BlockingQueue<CharSequence> intake, store;
private int oddWord; //Counter for odd words
/**
* Constructor for Word Reverser
*
* @param intake the blocking queue to retrieve words from
* @param store the blocking queue to store the words
*/
public WordReverser(BlockingQueue intake, BlockingQueue store) {
WordReverser.intake = intake;
WordReverser.store = store;
oddWord = 0;
}
/**
* Called when being executed. Reverses a word by taking from intake and
* places the reversed word into store
*/
@Override
public void run() {
boolean isInterrupted = false;
while (!isInterrupted) {
try {
StringBuilder str = new StringBuilder(intake.take());
//Exit condition
if (str.toString().equalsIgnoreCase(END_FLAG)) {
Thread.currentThread().interrupt();
}
//If it is a word to be reversed, then reverse it
if (oddWord % 2 == 1) {
str = reverseWord(str);
}
//Put word in queue and increment counter
store.put(str);
++oddWord;
} catch (InterruptedException ex) {
isInterrupted = true;
}
}
//Puts pill into queue when main body is done
try {
store.put(END_FLAG);
} catch (InterruptedException ex) {
System.err.printf("Error setting flag in store.%nWordReverser%n%s%n", ex);
}
}
/**
* Reverses a word, leaving behind punctuation if there is any
*
* @param word the word to reverse
* @return a string builder object containing the reversed word
*/
private StringBuilder reverseWord(StringBuilder word) {
char punct = Character.MAX_VALUE;
//If has punctuation at the end, remove the punctuation
if (!Character.isLetterOrDigit(word.charAt(word.length() - 1))) {
punct = word.charAt(word.length() - 1);
word.deleteCharAt(word.length() - 1);
}
word = word.reverse();
if (punct == Character.MAX_VALUE) {
return word;
}
return word.append(punct);
}
}
Lastly is the WordWriter: It writes a single word at a time to a file.
package ProducerConsumerAssignment;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.concurrent.BlockingQueue;
/**
* Takes a reversed word from the queue and writes it to a file
*
* 25/10/2014
*
* @author Tyler Weaver
*/
public class WordWriter implements Runnable {
private final String END_FLAG = "Terminate the queue";
//^Pill for the Poison Pill Technique. Works because every String sent
//through the queue will always be one word. Because this is multiple words
//There is no way another string could have the same value.
private static BlockingQueue<CharSequence> in;
private final File output;
/**
* Constructs a WordWriter object
*
* @param file the file to write words to
* @param queue the blocking queue to retrieve words from
*/
public WordWriter(final File file, BlockingQueue queue) {
output = file;
in = queue;
}
/**
* Executes when being called in a thread
*/
@Override
public void run() {
boolean isInterrupted = false;
try (BufferedWriter writer = new BufferedWriter(new FileWriter(output))) {
//Continue writing until the thread is interrupted
while (!isInterrupted) {
CharSequence word = in.take();
if (word.toString().equalsIgnoreCase(END_FLAG)) {
isInterrupted = true;
} else {
writer.write(word + " ");
}
}
} catch (InterruptedException | IOException ex) {
System.err.printf("Error writing to output file!%n%s%n", ex);
}
}
}
