Skip to main content
Commonmark migration
Source Link

##MessageViewerProgram

MessageViewerProgram

##MessageWriterProgram

MessageWriterProgram

##MessageViewerProgram

##MessageWriterProgram

MessageViewerProgram

MessageWriterProgram

Source Link
Doi9t
  • 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);
}
  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;
      }
   }
}