5
\$\begingroup\$

I recently created a program that could get information from a client, a name and some text, and then put that into a text file. From there the user can get open the text file straight from the program. I made it in two days. Please give me some tips in ways I can improve it, thank you!

The Server:

import java.io.*;
import java.net.*;
import java.nio.*;
import java.nio.channels.FileChannel;
import java.nio.file.*;

public class Server {
static boolean isAvailable = false;

public static void main(String[] args) throws IOException {

    // Get both connections to the client
    ServerSocket serverSocket =
            new ServerSocket(4999);
    ServerSocket serverSocket2 =
            new ServerSocket(4998);
    Socket socket =
            serverSocket.accept();
    Socket socket2 =
            serverSocket2.accept();

    // Confirm client connection
    System.out.println("The client has been connected to the server");

    // Get the streams from the client containing the string
    InputStreamReader fileNameIn =
            new InputStreamReader(socket.getInputStream());
    BufferedReader br =
            new BufferedReader(fileNameIn);

    InputStreamReader informationIn =
            new InputStreamReader(socket2.getInputStream());
    BufferedReader br2 =
            new BufferedReader(informationIn);

    // Read and assign the strings
    String fileName =
            br.readLine();
    String fileInformation =
            br2.readLine();

    // Get char array to write to the created file
    char[] fileInformationC =
            fileInformation.toCharArray();

    // Create file and write to it
    try (FileChannel fileChannel =
                 (FileChannel) Files.newByteChannel(
                         Path.of("C:\\Users\\Emman\\" + fileName + ".txt"),
                         StandardOpenOption.CREATE,
                         StandardOpenOption.WRITE
                 )) {

        int allocatedVal = 1000;

        ByteBuffer byteBuffer =
                ByteBuffer.allocate(allocatedVal);


        for (char c : fileInformationC){
            byteBuffer.put((byte) c);

            // Error handling: it is now impossible for the Buffer to run out of space
            // However a BufferOverflowException is caught, in case of a problem
            if (!(byteBuffer.position() == allocatedVal)){
                ByteBuffer.allocate(allocatedVal * 2);
            }
        }

        byteBuffer.rewind();

        fileChannel.write(byteBuffer);

        System.out.println("File has been created and written to, \n" +
                "Press Enter to open file");

        // The input does not matter
        //noinspection ResultOfMethodCallIgnored
        System.in.read();
        ProcessBuilder processBuilder =
                new ProcessBuilder("notepad.exe", "C:\\Users\\Emman\\" + fileName + ".txt");
        processBuilder.start();
    } catch (InvalidPathException e) {
        isAvailable = true;
        PrintWriter printToClient =
                new PrintWriter(socket.getOutputStream());

        printToClient.write("\"Error in creating/writing to file \\n\" +\n" +
                "Due to incorrect/invalid path\" +\n" +
                "e");
        printToClient.close();
    } catch (IOException e) {
        isAvailable = true;
        PrintWriter printToClient =
                new PrintWriter(socket.getOutputStream());

        printToClient.write("Error in creating/writing to file \n" +
                "Due to error in IO" +
                e);
        printToClient.close();

    } catch (BufferOverflowException e) {
        isAvailable = true;
        PrintWriter printToClient =
                new PrintWriter(socket.getOutputStream());

        printToClient.write("Error in creating/writing to file \n" +
                "Due to a buffer overflow" +
                e);
        printToClient.close();
    }

}
}

The Client:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.Socket;

public class Client {
public static void main(String[] args) throws IOException{

    // Create connections to server
    Socket socket =
            new Socket("localhost", 4999);
    Socket socket2 =
            new Socket("localhost", 4998);
    // Create print writer output stream to give to server
    PrintWriter pr =
            new PrintWriter(socket.getOutputStream());
    PrintWriter pr2 =
            new PrintWriter(socket2.getOutputStream());

    // Create a buffered reader for reading the file name and information
    BufferedReader br =
            new BufferedReader(
                    new InputStreamReader(
                            System.in
                    )
            );

    // Get the file names and information
    String fileName = br.readLine();

    String information = br.readLine();

    // Put in the file Name and the file information
    pr.write(fileName);
    pr.close();

    pr2.write(information);
    pr2.close();

    // Checking whether the error message is available,
    // if not only one line of code is read, wasting barely any time
    if(Server.isAvailable){
        InputStreamReader errorInfoIn =
                new InputStreamReader(socket.getInputStream());
        BufferedReader readErrorInfo =
                new BufferedReader(errorInfoIn);

        String errorInfo =
                readErrorInfo.readLine();
        System.out.println(errorInfo);
        errorInfoIn.close();
    }
}
}
\$\endgroup\$
1
\$\begingroup\$

Don't put everything in a single function. That makes your code difficult to maintain and reuse, even by yourself. The ideal function size is so small that one can no longer reasonably extract further functions from it. In your code, each program is just one function, quite the opposite of the ideal.

All resources should be handled with try-with-resources constructs, not only the fileChannel.

It is not possible for a reviewer to run your program without changing it first, because you use an absolute, operating-system-specific and user-specific path in your program: Path.of("C:\\Users\\Emman\\" + fileName + ".txt").

If you're serious about learning network programming, you may want to do some sanitation to fileName. What if it contains ..? What if somebody sends ..\..\WINNT\explorer.exe or something like that as filename?

The way how your program is written, isAvailable can and should be a local variable. In general, having non-final static variables is considered bad design. It prevents multiple instances of your class to function independently in parallel.

| improve this answer | |
\$\endgroup\$

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.