Problem
I am writing a textual chat app for exercise purposes. All Chatters using my program have access to a network directory, and there my program is stored. This chat app consists of three programs. The user management program allows you to create and delete users. The messaging program allows you to write messages. The viewer program shows all messages.
Now I am about to find a strategy that is good for my viewer program. The code is not ordered yet, I just want to ask if the logic that is behind my code is suitable for a chat viewing program, or if there is a more sophisticated way to create such programs.
import java.io.FileInputStream;
import java.io.ObjectInputStream;
import java.io.IOException;
import java.io.File;
public class MessageViewerProgram {
public static void main(String[] args) throws Exception {
String saveDir = ".save/messages/";
int fileName = 1;
File file = new File(saveDir + fileName);
while (true) {
Thread.sleep(50);
if (file.exists()) {
Thread.sleep(20);
try (FileInputStream fis = new FileInputStream(file);
ObjectInputStream ois = new ObjectInputStream(fis)) {
Message message = (Message) ois.readObject();
System.out.println(message.getSenderName() + ": " + message.getMessage());
fileName++;
file = new File(saveDir + fileName);
} catch (IOException e) {
e.printStackTrace();
} catch (ClassNotFoundException e) {
e.printStackTrace();
}
}
}
}
}
And this here is my “MessageWriterProgram”. It is not finished yet, but it works already. I just post it because maybe it is needed to understand the problem, but you dont have to review the following class:
import java.util.Scanner;
import java.io.File;
import java.io.FileOutputStream;
import java.io.ObjectOutputStream;
import java.io.IOException;
public class MessageWriterProgram {
public static void main(String[] args) {
MessageWriterProgram mwp = new MessageWriterProgram();
mwp.mainLoop();
}
private static final String SAVE_DIR_NAME = ".save/messages/";
private Scanner scanner = new Scanner(System.in);
private UserManager userManager;
private User identity;
public MessageWriterProgram() {
scanner = new Scanner(System.in);
userManager = new UserManager();
}
public void mainLoop() {
if (!login()) {
System.out.println("Login was not successful.");
return;
}
System.out.println(identity.getName());
while (true) {
System.out.print("> ");
String input = scanner.nextLine();
if (!input.isEmpty()) {
Message message = new Message(identity.getName(), input);
saveMessage(message);
}
}
}
public void saveMessage(Message message) {
// create file name that isn't used
int filename = 1;
while (true) {
File file = new File(SAVE_DIR_NAME + filename);
if (file.exists()) {
filename++;
} else {
break;
}
}
File file = new File(SAVE_DIR_NAME + filename);
// store message
try (FileOutputStream fos = new FileOutputStream(file);
ObjectOutputStream oos = new ObjectOutputStream(fos)) {
oos.writeObject(message);
} catch (IOException e) {
e.printStackTrace();
}
}
// returns if login was successful
public boolean login() {
System.out.print("Name: ");
String name = scanner.nextLine();
System.out.print("Password: ");
String password = scanner.nextLine();
identity = userManager.retainUserIdentity(name, password);
return identity != null;
}
}
Solution
I have some suggestions.
MessageViewerProgram
- Instead of brute forcing all the messages in the directory, I suggest that you list them and iterate on them instead. You can use the method
java.io.File#listFiles()
for this.
File file = new File(saveDir);
File[] files = file.listFiles();
This will give you an array of the files in the message folder, and you can iterate.
File saveDir = new File(".save/messages/");
for (File currentMessageFile : saveDir.listFiles()) {
try (FileInputStream fis = new FileInputStream(currentMessageFile);
ObjectInputStream ois = new ObjectInputStream(fis)) {
Message message = (Message) ois.readObject();
System.out.println(message.getSenderName() + ": " + message.getMessage());
} catch (IOException e) {
e.printStackTrace();
} catch (ClassNotFoundException e) {
e.printStackTrace();
}
}
MessageWriterProgram
- The initialization of the
MessageWriterProgram#scanner
variable outside the constructor is useless, it’s created inside the constructor already.
private Scanner scanner;
public MessageWriterProgram() {
scanner = new Scanner(System.in);
}
- I suggest that you rename the method
mainLoop
instart
orrun
or evenexecute
.
MessageWriterProgram#saveMessage
method
-
Rename the variable
filename
intofileName
-
I suggest that you extract the logic of counting the next file name in a method.
public void saveMessage(Message message) {
// create file name that isn't used
int fileName = getNextFilename();
//[...]
}
private int getNextFilename() {
int fileName = getNextFilename();
while (true) {
File file = new File(SAVE_DIR_NAME + fileName);
if (file.exists()) {
fileName++;
} else {
break;
}
}
return fileName;
}
- Instead of counting the messages in the loop, you can use the
java.io.File#listFiles()
and check the number of files in the folder.
private int getNextFilename() {
File messageDirectory = new File(SAVE_DIR_NAME);
File[] files = messageDirectory.listFiles();
if (files != null) {
return files.length + 1;
} else {
return 1;
}
}
MessageWriterProgram#login
method
In my opinion, this method is a bit useless and add confusion, since it does more than one thing.
- Check if the User is present.
- Update the
identity
variable.
If you inline it, it will be more readable and you can convert the identity
variable to a local variable.
System.out.print("Name: ");
String name = scanner.nextLine();
System.out.print("Password: ");
String password = scanner.nextLine();
User identity = userManager.retainUserIdentity(name, password);
if (identity == null) {
System.out.println("Login was not successful.");
return;
}
Refactored MessageWriterProgram
class
public class MessageWriterProgram {
public static void main(String[] args) {
MessageWriterProgram mwp = new MessageWriterProgram();
mwp.mainLoop();
}
private static final String SAVE_DIR_NAME = ".save/messages/";
private Scanner scanner;
private UserManager userManager;
public MessageWriterProgram() {
scanner = new Scanner(System.in);
userManager = new UserManager();
}
public void mainLoop() {
System.out.print("Name: ");
String name = scanner.nextLine();
System.out.print("Password: ");
String password = scanner.nextLine();
User identity = userManager.retainUserIdentity(name, password);
if (identity == null) {
System.out.println("Login was not successful.");
return;
}
System.out.println(identity.getName());
while (true) {
System.out.print("> ");
String input = scanner.nextLine();
if (!input.isEmpty()) {
Message message = new Message(identity.getName(), input);
saveMessage(message);
}
}
}
public void saveMessage(Message message) {
// create file name that isn't used
int fileName = getNextFilename();
File file = new File(SAVE_DIR_NAME + fileName);
// store message
try (FileOutputStream fos = new FileOutputStream(file);
ObjectOutputStream oos = new ObjectOutputStream(fos)) {
oos.writeObject(message);
} catch (IOException e) {
e.printStackTrace();
}
}
private int getNextFilename() {
File messageDirectory = new File(SAVE_DIR_NAME);
File[] files = messageDirectory.listFiles();
if (files != null) {
return files.length + 1;
} else {
return 1;
}
}
}