Return to Answer

• 1

• 3.4k
• 3
• 11
• 23

I have some suggestions.

##MessageViewerProgram

1. 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

1. 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);
}

2. I suggest that you rename the method mainLoop in start or run or even execute.

MessageWriterProgram#saveMessage method

1. Rename the variable filename into fileName

2. 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;
}

3. 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;
 }
 }
}