4
\$\begingroup\$

I have rewritten my little chat application and tried to pay attention to former hints and tips. The viewer and the writer have to run as seperate applications. the Writer.class basically stores messages as property files. The directory, where the files get stored, is watched by the Viewer.class and gets scanned every 0.5 seconds.

I tried to implement a clear structure to my source code and to implement clean methods with clear names and unique resposibilities. I also tried to write clear comments.

What do you think about my program? Is there something that I can improve?

Viewer.java

import java.util.Arrays;
import java.util.List;
import java.util.ArrayList;
import java.util.Date;
import java.util.Properties;
import java.io.File;
import java.io.BufferedInputStream;
import java.io.FileInputStream;
import java.io.IOException;

public class Viewer {
    public static void main(String[] args) {
        Viewer viewer = new Viewer();
        viewer.view();
    }
    
    // contains all messages within two sub-dirs
    private File saveDir;
    // contains messages that have to be displayed
    private File currentDir;
    // contains messages that don't have to be displayed
    private File archiveDir;
    // determines the maximal amount of messages in the current directory
    private int currentMessagesCount;
    // last message read into program
    private File lastMessage;
    
    public Viewer() {
        // initialize members from file named config
        try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream("config"))) {
            Properties config = new Properties();
            config.load(stream);
            currentMessagesCount = Integer.parseInt(config.getProperty("current_messages_count"));
            saveDir = new File(config.getProperty("save_dir"));
        } catch (IOException e) {
            e.printStackTrace();
        }
        
        currentDir = new File(saveDir, "current");
        archiveDir = new File(saveDir, "archive");
        lastMessage = new File("no last message");
        
        // create save directories if don't exist
        if (!saveDir.exists()) {
            saveDir.mkdir();
        }
        if (!currentDir.exists()) {
            currentDir.mkdir();
        }
        if (!archiveDir.exists()) {
            archiveDir.mkdir();
        }
        
        File[] messages = currentDir.listFiles();
        Arrays.sort(messages);
    }
    
    public void view() {
        printAllMessages();
        while (true) {
            try {
                Thread.sleep(500);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            printNewMessage();
            checkLimit();
        }
    }
    
    // prints all messages immediately
    private void printAllMessages() {
        File[] messages = currentDir.listFiles();
        
        if (messages.length == 0) {
            return;
        }
        
        Arrays.sort(messages);
        for (int i = 0; i < messages.length; i++) {
            printMessage(messages[i]);
        }
        
        lastMessage = messages[messages.length - 1];
    }
    
    // prints newest if there is a more recent than last read message
    private void printNewMessage() {
        File[] messages = currentDir.listFiles();
        Arrays.sort(messages);
        
        if (messages.length == 0) {
            return;
        }
        
        // add all new messages to a list and print it out reversively
        List<File> newMessages = new ArrayList<>();
        for (int i = messages.length - 1; i >= 0 ; i--) {
            if (!messages[i].toString().equals(lastMessage.toString())) {
                newMessages.add(messages[i]);
            } else {
                break;
            }
        }
        
        while (newMessages.size() > 0) {
            File currentMessage = newMessages.get(newMessages.size() - 1);
            
            printMessage(currentMessage);
            newMessages.remove(currentMessage);
            lastMessage = currentMessage;
        }
        
    }
    
    private void printMessage(File file) {
        try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(file))) {
            Properties currentMessage = new Properties();
            currentMessage.load(stream);
            System.out.println(currentMessage.getProperty("date") + " (" + currentMessage.getProperty("nickname") + ")");
            System.out.println(currentMessage.getProperty("content") + "\n");
        } catch (IOException e) {
            e.printStackTrace();
        }       
    }
    
    // checks if there are too much messages in currentDir
    private void checkLimit() {
        File[] messages = currentDir.listFiles();
        Arrays.sort(messages);
        int numOfSuperfluousMessages = messages.length - currentMessagesCount;
        if (numOfSuperfluousMessages > 0) {
            for (int i = 0; i < numOfSuperfluousMessages; i++) {
                messages[i].renameTo(new File(archiveDir, messages[i].getName()));
            }
        }
    }
}

Writer.java

import java.util.Scanner;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileWriter;
import java.io.BufferedWriter;
import java.io.IOException;
import java.util.Calendar;
import java.util.Date;
import java.util.Properties;

public class Writer {
    public static void main(String[] args) {
        System.out.print("Your nickname: ");
        String nickname = scanner.nextLine();
        
        Writer writer = new Writer(nickname);
        writer.chat();
    }
    
    private static Scanner scanner = new Scanner(System.in);
    private String nickname;
    // directory in which messages get stored
    // initialized by file named config
    private File currentDir;
    
    public Writer(String nickname) {
        this.nickname = nickname;
        try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream("config"))) {
            Properties config = new Properties();
            config.load(stream);
            currentDir = new File(config.getProperty("save_dir"), "current");
        } catch (IOException e) {
            e.printStackTrace();
        }
        
        // create current dir if not exists
        if (!currentDir.exists()) {
            currentDir.mkdirs();
        }
    }
    
    public void chat() {
        while (true) {
            System.out.print(nickname + " > ");
            String newContent = scanner.nextLine();
            long millis_bygone = new Date().getTime();
            
            File newMessage = new File(currentDir, new Long(millis_bygone).toString());
            
            // create file
            try {
                newMessage.createNewFile();
            } catch (IOException e) {
                e.printStackTrace();
            }
            
            // store message to file in currentDir
            try (BufferedWriter br = new BufferedWriter(new FileWriter(newMessage))) {
                br.write("nickname: " + nickname + "\n");
                
                // create date string
                Calendar cal = Calendar.getInstance();
                StringBuilder dateString = new StringBuilder();
                dateString.append(cal.get(Calendar.DAY_OF_MONTH) + "." + (cal.get(Calendar.MONTH) + 1) + "." + cal.get(Calendar.YEAR) + " ");
                dateString.append(cal.get(Calendar.HOUR_OF_DAY) + ":" + cal.get(Calendar.MINUTE));
                br.write("date: " + dateString + "\n");
                
                br.write("content: " + newContent + "\n");
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

Very good implementation, commented and easy to understand. There is no much to improve but I am providing you my suggestions.

Configuration

  • Viewer and Writer cannot start without the configuration file, in that case it's better to provide a message to the user and exit gracefully
  • The logic to read the configuration file is duplicated, so it's better to move it in its own class

Exception handling

  • There are many operations with files but the exceptions are ignored

In your case you could handle exceptions in two ways: stop the application or ignore it. For example if the Viewer cannot read a message from a file is better to stop the application:

try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(file))) {
// ...
} catch (IOException e) {
    throw new RuntimeException("Error: unable to read the message in: "+file.getName());
}

The main will catch the runtime exception, print the error message and exit.

public static void main(String[] args) {
    try {
        Viewer viewer = new Viewer();
        viewer.view();
    } catch (Exception e) {
        System.out.print("Error: " + e.getMessage());
    }
}

The second way is easier, for example if the Writer cannot write to a file we can just print a warning and continue:

try (BufferedWriter br = new BufferedWriter(new FileWriter(newMessage))) {
// ...
} catch (IOException e) {
    System.out.println("Warning: cannot write to file " + newMessage);
}

Encapsulation

The chat message has its own format: a nickname (sender), a date and content. But this format is not clearly represented in a schema. Creating a class for the message will make it more evident and easier to change or extend.

Having more classes also means that you will need to build a jar file instead of running the .class directly. If your requirement is to keep the whole application inside a single .class file then you can put more classes in the same file.


Mismatch between name and behaviour

The method printMessage does more than printing a message to the console, it reads a file, parse it and finally prints it to console. It can be improved by splitting that logic.


Minor changes

  • Use Java 8 LocalDateTime and Instant instead of Date (more readable)
  • Let the user quit the chat in order to gracefully exit and close the Scanner

This is the code refactored:

public class ConfigFileReader {
    private String saveDir;
    private Integer currentMessagesCount;
    
    public ConfigFileReader(String fileName) {
        Properties config;
        try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(fileName))) {
            config = new Properties();
            config.load(stream);
        } catch (IOException e) {
            throw new RuntimeException("Cannot read config file");
        }
        // Read save_dir property or set it to the local folder if not found
        saveDir = config.getProperty("save_dir",".");
        // Read current_messages_count property or set it to 1 if not found
        currentMessagesCount = Integer.parseInt(config.getProperty("current_messages_count","1"));
    }
    
    public String getSaveDir() {
        return saveDir;
    }

    public Integer getCurrentMessagesCount() {
        return currentMessagesCount;
    }
    
}

The ChatMessage class:

public class ChatMessage {
    private String sender;
    private String content;
    private String date;
    
    public ChatMessage(String sender, String content) {
        this.sender = sender;
        this.content = content;
        this.date = now();
    }
    
    public ChatMessage(String sender, String content, String date) {
        this.sender = sender;
        this.date = date;
        this.content = content;
    }
    
    // Returns current date and time
    private String now() {      
        return LocalDateTime.now()
                .format(DateTimeFormatter.ofPattern("dd.MM.yyyy HH:mm"));
    }

    // Write message to file in currentDir
    public void saveToFile(File currentDir, String fileName) {
        File newMessage = new File(currentDir, fileName);
        // Create file
        try {
            newMessage.createNewFile();
        } catch (IOException e) {
            throw new RuntimeException("Cannot create file "+ fileName);
        }
        // Write message in the file
        try (BufferedWriter br = new BufferedWriter(new FileWriter(newMessage))) {
            br.write("nickname: " + this.sender + "\n");
            br.write("date: " + this.date + "\n");
            br.write("content: " + this.content + "\n");
        } catch (IOException e) {
            throw new RuntimeException("Cannot write to file "+ fileName);
        }
    }
    
    public static ChatMessage fromFile(File file) {
        ChatMessage result = null;
        try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(file))) {
            Properties currentMessage = new Properties();
            currentMessage.load(stream);
            String date = currentMessage.getProperty("date");
            String nickname = currentMessage.getProperty("nickname");
            String content = currentMessage.getProperty("content");
            result = new ChatMessage(nickname,content,date);
        } catch (IOException e) {
            throw new RuntimeException("Error: unable to read the message in: "+file.getName());
        }
        return result;
    }

    public String getSender() {
        return sender;
    }

    public String getDate() {
        return date;
    }

    public String getContent() {
        return content;
    }
}

The Writer:

public class Writer {
    public static void main(String[] args) {
        System.out.print("Your nickname: ");
        String nickname = scanner.nextLine();

        try {
            new Writer(nickname).chat();
        } catch (Exception e) {
            System.out.print("Error: " + e.getMessage());
        } finally {
            scanner.close();
        }
    }

    private static Scanner scanner = new Scanner(System.in);
    private String nickname;
    // directory in which messages get stored
    // initialized by file named config
    private File currentDir;

    public Writer(String nickname) {
        this.nickname = nickname;
        String saveDir = new ConfigFileReader("config").getSaveDir();

        currentDir = new File(saveDir, "current");

        // create current dir if not exists
        if (!currentDir.exists()) {
            currentDir.mkdirs();
        }
    }

    public void chat() {
        System.out.println("Start chatting or type quit to exit.");
        while (true) {
            System.out.print(nickname + " > ");
            String newContent = scanner.nextLine();

            if (newContent.strip().equalsIgnoreCase("quit"))
                break;

            // long millis_bygone = new Date().getTime();
            // String fileName = new Long(millis_bygone).toString();
            // Java 8
            String fileName = String.valueOf(Instant.now().toEpochMilli());

            ChatMessage message = new ChatMessage(nickname, newContent);
            try {
                message.saveToFile(currentDir, fileName);
            } catch (Exception e) {
                System.out.println("Warning: cannot write to file " + fileName);
            }
        }
    }
}

And finally the Viewer:

public class Viewer {
    public static void main(String[] args) {
        try {
            new Viewer().view();
        } catch (Exception e) {
            System.out.print("Error: " + e.getMessage());
        }
    }

    // contains all messages within two sub-dirs
    private File saveDir;
    // contains messages that have to be displayed
    private File currentDir;
    // contains messages that don't have to be displayed
    private File archiveDir;
    // determines the maximal amount of messages in the current directory
    private int currentMessagesCount;
    // last message read into program
    private File lastMessage;

    public Viewer() {
        // Read configuration file
        ConfigFileReader cfReader = new ConfigFileReader("config");
        
        currentMessagesCount = cfReader.getCurrentMessagesCount();
        saveDir = new File(cfReader.getSaveDir());

        currentDir = new File(saveDir, "current");
        archiveDir = new File(saveDir, "archive");
        lastMessage = new File("no last message");

        // create save directories if don't exist
        if (!saveDir.exists()) {
            saveDir.mkdir();
        }
        if (!currentDir.exists()) {
            currentDir.mkdir();
        }
        if (!archiveDir.exists()) {
            archiveDir.mkdir();
        }

        File[] messages = currentDir.listFiles();
        Arrays.sort(messages);
    }

    public void view() {
        printAllMessages();
        while (true) {
            try {
                Thread.sleep(500);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            printNewMessage();
            checkLimit();
        }
    }

    // prints all messages immediately
    private void printAllMessages() {
        File[] messages = currentDir.listFiles();

        if (messages.length == 0) {
            return;
        }

        Arrays.sort(messages);
        for (int i = 0; i < messages.length; i++) {
            ChatMessage message = ChatMessage.fromFile(messages[i]);
            printMessage(message);
        }

        lastMessage = messages[messages.length - 1];
    }

    // prints newest if there is a more recent than last read message
    private void printNewMessage() {
        File[] messages = currentDir.listFiles();
        Arrays.sort(messages);

        if (messages.length == 0) {
            return;
        }

        // add all new messages to a list and print it out reversively
        List<File> newMessages = new ArrayList<>();
        for (int i = messages.length - 1; i >= 0; i--) {
            if (!messages[i].toString().equals(lastMessage.toString())) {
                newMessages.add(messages[i]);
            } else {
                break;
            }
        }

        while (newMessages.size() > 0) {
            File currentMessage = newMessages.get(newMessages.size() - 1);
            ChatMessage message = ChatMessage.fromFile(currentMessage);
            printMessage(message);
            newMessages.remove(currentMessage);
            lastMessage = currentMessage;
        }

    }

    private void printMessage(ChatMessage message) {
        System.out.println(message.getDate() + " (" + message.getSender() + ")");
        System.out.println(message.getContent());
        System.out.println();
    }

    // checks if there are too much messages in currentDir
    private void checkLimit() {
        File[] messages = currentDir.listFiles();
        Arrays.sort(messages);
        int numOfSuperfluousMessages = messages.length - currentMessagesCount;
        if (numOfSuperfluousMessages > 0) {
            for (int i = 0; i < numOfSuperfluousMessages; i++) {
                messages[i].renameTo(new File(archiveDir, messages[i].getName()));
            }
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for your great work! \$\endgroup\$ – Dexter Thorn Jul 17 '20 at 7:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.