I was given an assignment to write all ordered contents of given files into a result.txt. At first, the filenames are split into different Arraylists where each file contains a label in a format #n/N where N is the total number of files. e.g.
British explorer James Clark Ross led the first expedition to reach the north magnetic pole #001/004
from a file 1831-06-01.txt
The problem with my code is that it has written in order 1,4,2,3 respectively. However, the result must be in order 1,2,3,4. This may be due to a lack of synchronization. Nonetheless, I am still struggling to fix the problem.
This is my code:
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.util.*;
class PopThread implements Runnable {
ArrayList<String> fileList;
public PopThread(ArrayList<String> fileList) {
this.fileList = fileList;
}
@Override
public void run() {
//System.out.println("running\n");
Thread.currentThread().setPriority(Thread.MIN_PRIORITY);
long startTime = System.nanoTime();
System.out.println("fileList: " fileList);
ArrayList<String> sortedFileList = sortFiles(fileList);
File resultFile = new File("result.txt");
for (String filename : sortedFileList) {
Writer w1 = new Writer(filename, resultFile);
Thread t = new Thread(w1);
t.setPriority(Thread.MAX_PRIORITY);
t.start();
}
long stopTime = System.nanoTime();
//System.out.println("Total execution time: " (stopTime - startTime));
}
public ArrayList<String> readFiles(String filename) {
ArrayList<String> list = new ArrayList<String>();
try {
File myObj = new File(filename);
Scanner s = new Scanner(myObj);
while (s.hasNext()) {
list.add(s.next());
}
s.close();
} catch (FileNotFoundException e) {
e.printStackTrace();
}
return list;
}
public int getNumber(String filename) {
String lastLine = "";
String sCurrentLine;
int identifier_integer = -1;
try {
BufferedReader br = new BufferedReader(new FileReader(filename));
while ((sCurrentLine = br.readLine()) != null) {
lastLine = sCurrentLine;
}
String identifier_number = lastLine.substring(1,4);
identifier_integer = Integer.parseInt(identifier_number);
} catch (FileNotFoundException e) {
e.printStackTrace();
}
catch (IOException e) {
e.printStackTrace();
}
return identifier_integer;
}
public ArrayList<String> sortFiles(ArrayList<String> listFileName) {
int i = listFileName.size();
boolean sorted = false;
while ( (i > 1) && (!(sorted)) ) {
sorted = true;
for (int j = 1; j < i; j ) {
if ( getNumber(listFileName.get(j-1)) > getNumber(listFileName.get(j)) ) {
String temp = listFileName.get(j-1);
listFileName.set(j-1, listFileName.get(j));
listFileName.set(j, temp);
sorted = false;
}
}
i--;
}
return listFileName;
}
}
class Writer implements Runnable {
String filename;
File resultFile;
public Writer(String filename, File resultFile) {
this.filename = filename;
this.resultFile = resultFile;
}
@Override
public void run() {
String content;
content = readFromFile(filename);
writeToFile(resultFile, content);
}
private static void writeToFile(File resultFile, String content) {
try {
BufferedWriter writer = new BufferedWriter(new FileWriter(resultFile, true));
writer.write(content);
//writer.write("file content written");
writer.flush();
} catch (IOException e) {
e.printStackTrace();
}
}
static String readFromFile(String filename) {
StringBuffer content = new StringBuffer();
try {
String text;
BufferedReader reader = new BufferedReader(new FileReader(filename));
while ((text = reader.readLine()) != null) {
content.append(text);
content.append("\n");
}
} catch (FileNotFoundException e) {
e.printStackTrace();
}
catch (IOException e) {
e.printStackTrace();
}
return content.toString();
}
}
public class q4 {
public static void main(String[] args) {
ArrayList<String> filesOne = new ArrayList<String>();
filesOne.add("1831-06-01.txt");
filesOne.add("2003-08-27.txt");
ArrayList<String> filesTwo = new ArrayList<String>();
filesTwo.add("1961-04-12.txt");
filesTwo.add("1972-12-11.txt");
PopThread popRunnableOne = new PopThread(filesOne);
PopThread popRunnableTwo = new PopThread(filesTwo);
Thread threadOne = new Thread(popRunnableOne);
Thread threadTwo = new Thread(popRunnableTwo);
threadOne.start();
threadTwo.start();
try {
threadOne.join();
threadTwo.join();
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
( NOTE: The class q4 cannot be altered)
CodePudding user response:
This assignment is horrible. You have my sympathy.
Your two threads will have to communicate with each other. Each thread will have to know, what is the filename that the other thread wants to output next. And, they will have to take turns. Each thread needs to loop:
While the date on my next file is less than or equal to the date on the other thread's next file, output my next file,
Tell the other thread, "it's your turn,"
If I have no more files, then exit (return from the
run()
method), otherwise, wait for the other thread to tell me it's my turn again,Go back to step 1.
Having to take turns is the worst part of the assignment. Any time you find yourself needing to make threads take turns doing something—any time you need to make threads do things in a particular order—that's a clear sign that all of the things should be done by a single thread.
The only way threads can communicate is through shared variables. Your instructor has done you a huge disservice by telling you not to modify the q4
class. That prevents you from passing any shared objects in to your PopThread
implementation through its constructor.
The only other way your two threads can share any variables is by making the variables static
. Forcing you to use static
is the second worst part of the assignment. If you go on to study software engineering, you will learn that static
is an anti-pattern. Programs that use static
variables are brittle (i.e., hard to modify), and they are hard to test.
Forcing you to use static variables also will make your threads do extra work to figure out who is who. Normally, I would do something like this so that each thread would automatically know which state is its own, and which belongs to the other guy:
class SharedState { ... }
class PopThread {
public PopThread(
SharedState myState,
SharedState otherThreadState,
ArrayList<String> fileList
) {
this.myState = myState;
this.otherGuyState = otherThreadState;
this.fileList = fileList;
...initialize this.myState...
}
...
}
class q4 {
public static void main(String[] args) {
SharedState stateOne = new SharedState();
SharedState stateTwo = new SharedState();
PopThread popRunnableOne = new PopThread(stateOne, stateTwo, filesOne);
PopThread popRunnableTwo = new PopThread(stateTwo, stateOne, filesTwo);
...
}
}
The best way I can think of with static variables would be to have an array of two SharedState
, and have the threads use an AtomicInteger
to each assign themself one of the two array slots:
class PopThread {
static SharedState[] state = new SharedState [2];
static AtomicInteger nextStateIndex = new AtomicInteger(0);
public PopThread(
SharedState myState,
SharedState otherThreadState,
ArrayList<String> fileList
) {
myStateIndex = nextStateIndex.getAndIncrement();
otherGuysStateIndex = myStateIndex ^ 1;
this.fileList = fileList;
...initialize state[myStateIndex]...
}
...
}