3
\$\begingroup\$

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;
 }
}
asked Jan 25, 2020 at 19:16
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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);
}
  1. 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;
}
  1. 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;
 }
 }
}
answered Jan 26, 2020 at 0:32
\$\endgroup\$
1
  • \$\begingroup\$ That's an excellent code review, just wow! \$\endgroup\$ Commented Jan 28, 2020 at 17:38

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.