What would be considered better, cleaner code from the following options? The code basically sends a file through a Socket using an OutputStream that encrypts it. The code does not look that basic when read but it does not need to be understood at a technical level, this is not the point of the question. The comments tell what is going on at a general level.
I am obviously not looking for an objective answer as design is usually very subjective and depends on many factors. I just want opinions that create constructive feedback around the problem.
Option 1: The one method way. I consider this WAY cleaner and easier to read, but obviously, it brakes many OO principles such as cohesion and modularity.
public class FileSender {
public static final String KEY = "00000000000000000000000000000000";
public static final String ALGORITHM = "AES";
public static final String FILE = "C:\\Users\\Andres\\Desktop\\file.txt";
public static final String HASH_ALG = "MD5";
public static void main(String [] args) throws Exception {
CipherOutputStream cipherOut = null;
FileInputStream fileReader = null;
Socket client = null;
FileInputStream fis = null;
try {
System.out.println("Running client ...");
//Connects to serverSocket.
client = new Socket("localhost", 8081);
//Creates MD5 Hash.
MessageDigest md = MessageDigest.getInstance(HASH_ALG);
File file = new File(FILE);
fis = new FileInputStream(file);
byte[] byteArray = new byte[1024];
int bytesCount = 0;
while ((bytesCount = fis.read(byteArray)) != -1)
md.update(byteArray, 0, bytesCount);
byte[] hash = md.digest();
String checksum = String.format("%032x",new BigInteger(1, hash));
fis.close();
//Creates "secret" key and inits the cipher.
SecretKeySpec keySpec = new SecretKeySpec(hex2byte(KEY), ALGORITHM);
Cipher cipher = Cipher.getInstance(ALGORITHM + "/ECB/PKCS5PADDING");
cipher.init(Cipher.ENCRYPT_MODE, keySpec);
cipherOut = new CipherOutputStream(client.getOutputStream(), cipher);
//Sends the file.
byte[] fileBuffer = new byte[1024];
fileReader = new FileInputStream(FILE);
int len;
while ((len = fileReader.read(fileBuffer)) != -1)
cipherOut.write(fileBuffer, 0, len);
System.out.println("Ended client .... ");
}catch(IOException e) {
e.printStackTrace();
}finally{
if(fileReader != null) fileReader.close();
if(cipherOut != null) cipherOut.close();
if(client != null) client.close();
if(fis != null) fis.close();
}
}
public static byte[] hex2byte(String s) {
return DatatypeConverter.parseHexBinary(s);
}
}
Option 2: The modular way, dividing the code. This is obviously the correct way in most cases because it cares about OO principles, but is the code cleaner?
public class FileSender {
public static final String KEY = "00000000000000000000000000000000";
public static final String ALGORITHM = "AES";
public static final String FILE = "C:\\Users\\Andres\\Desktop\\file.txt";
public static final String HASH_ALG = "MD5";
private String fileName;
private String serverMachine;
private int serverPort;
public FileSender(String fileName, String serverMachine, int serverPort){
this.fileName = fileName;
this.serverMachine = serverMachine;
this.serverPort = serverPort;
}
//High level method that uses the class details.
public void sendFile(){
CipherOutputStream cipherOut = null;
Socket client = null;
String checkSum;
try {
System.out.println("Running client ...");
client = connectToServer();
checkSum = getHashChecksum();
cipherOut = initCipherStream(client);
transferData(cipherOut);
} catch (Exception e) {
System.out.println(e.getMessage());
} finally {
}
}
//Creates MD5 checksum
private String getHashChecksum() {
FileInputStream fis = null;
try {
MessageDigest md = MessageDigest.getInstance(HASH_ALG);
File file = new File(this.fileName);
fis = new FileInputStream(file);
byte[] byteArray = new byte[1024];
int bytesCount = 0;
while ((bytesCount = fis.read(byteArray)) >= 0)
md.update(byteArray, 0, bytesCount);
byte[] hash = md.digest();
fis.close();
return String.format("%032x",new BigInteger(1, hash));
} catch (Exception e) {
e.printStackTrace();
throw new RuntimeException("Error in checksum");
} finally {
try {
fis.close();
} catch (IOException e) {
throw new RuntimeException(
"Error in checksum. Provided file can't be closed.");
}
}
}
//Connects client to server.
private Socket connectToServer() throws Exception{
try {
Socket client = new Socket(this.serverMachine, this.serverPort);
return client;
} catch (IOException e) {
throw new Exception("Server can't be reached");
}
}
//Creates output stream to send file.
private CipherOutputStream initCipherStream(Socket client) {
//Crea la llave.
SecretKeySpec keySpec = new SecretKeySpec(hex2byte(KEY), ALGORITHM);
Cipher cipher;
try {
cipher = Cipher.getInstance(ALGORITHM + "/ECB/PKCS5PADDING");
cipher.init(Cipher.ENCRYPT_MODE, keySpec);
return new CipherOutputStream(client.getOutputStream(), cipher);
} catch (Exception e) {
e.printStackTrace();
throw new RuntimeException("Error in cipher stream initialization.");
}
}
//Sends file
private void transferData(CipherOutputStream cipherOut) {
FileInputStream fileReader = null;
try {
//Missing: send MD5 hash. TODO
fileReader = new FileInputStream(this.fileName);
byte[] fileBuffer = new byte[1024];
int len;
while ((len = fileReader.read(fileBuffer)) != -1)
cipherOut.write(fileBuffer, 0, len);
} catch (Exception e) {
throw new RuntimeException("File " + this.fileName + " not found");
} finally{
try {
if(fileReader != null) fileReader.close();
} catch (IOException e) {
throw new RuntimeException("Socket stream can't be closed");
}
}
}
private byte[] hex2byte(String s) {
return DatatypeConverter.parseHexBinary(s);
}
public static void main(String [] args) throws Exception {
FileSender fileSender = new FileSender(FILE, "localhost", 8081);
fileSender.sendFile();
}
}
What do you think about either approach? Would you recommend another strategy to make better, more readable code?
