patternjavaMinor
File I/O, SHA1 sum and compression
Viewed 0 times
filecompressionsumandsha1
Problem
The function
Since I am learning the language, I am looking for general review. Is my code idiomatic Java code? Are there parts where I could make the code better?
```
package yas;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.zip.DeflaterOutputStream;
// import java.util.zip.Inflater;
public class Yas {
public static void main(String[] args) {
try {
hash_blob("yasar arabaci");
} catch (NoSuchAlgorithmException e) {
e.printStackTrace();
}
}
/**
* @param args
* @throws NoSuchAlgorithmException
*/
public static String hash_blob(String content) throws NoSuchAlgorithmException {
String hash_string; // hold sha1 hash
String input = "blob " + content.length() + '\0' + content;
// Step 1 is to compute sha1
MessageDigest mDigest = MessageDigest.getInstance("SHA1");
byte[] result = mDigest.digest(input.getBytes());
{ // to create namespace for StringBuilder
// This code block was borrowed from internet ;)
StringBuilder sb = new StringBuilder();
for (int i = 0; i < result.length; i++) {
sb.append(Integer.toString((result[i] & 0xff) + 0x100, 16).substring(1));
}
hash_string = sb.toString();
}
System.out.println("hash string is " + hash_string);
// Step 3 is to write to a file
File dest_file;
{
File yas_dir = new File(System.getProperty("user.dir"), ".yas");
File ob
hash_blog is inspired by "how git stores its objects". It is supposed to take a String, append a header to it, compute the SHA1 of it, then compress it and write it to a file. The first two characters of SHA1 is a directory name, and the rest of the characters are the file name.Since I am learning the language, I am looking for general review. Is my code idiomatic Java code? Are there parts where I could make the code better?
```
package yas;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.zip.DeflaterOutputStream;
// import java.util.zip.Inflater;
public class Yas {
public static void main(String[] args) {
try {
hash_blob("yasar arabaci");
} catch (NoSuchAlgorithmException e) {
e.printStackTrace();
}
}
/**
* @param args
* @throws NoSuchAlgorithmException
*/
public static String hash_blob(String content) throws NoSuchAlgorithmException {
String hash_string; // hold sha1 hash
String input = "blob " + content.length() + '\0' + content;
// Step 1 is to compute sha1
MessageDigest mDigest = MessageDigest.getInstance("SHA1");
byte[] result = mDigest.digest(input.getBytes());
{ // to create namespace for StringBuilder
// This code block was borrowed from internet ;)
StringBuilder sb = new StringBuilder();
for (int i = 0; i < result.length; i++) {
sb.append(Integer.toString((result[i] & 0xff) + 0x100, 16).substring(1));
}
hash_string = sb.toString();
}
System.out.println("hash string is " + hash_string);
// Step 3 is to write to a file
File dest_file;
{
File yas_dir = new File(System.getProperty("user.dir"), ".yas");
File ob
Solution
Problems :
Style :
like this :
- methods mkdirs() and createNewFile() return booleans to indicate whether they are successful. You should check and handle if they return false.
- IOException gets ignored, you print the stacktrace but clients have no way of knowing the method failed.
Style :
- This is one procedural block in which different concerns ( finding and creating files, directories, computing hash, converting byte[] to a String, ... ) are handled. Despite an attempt to limit the of variables scope with a block statement, variable scope is unclear and often wider than strictly needed. There is but one cure : refactor : extract method.
- variable naming does not follow Java conventions.
- Don't return a dummy return value, declare the method to have
voidas return value, and you don't need to return a value.
- Comments : step 1, step 3 what happened to step 2?
printStackTrace()statement ends with ;; only one is needed. (probably a typo)
- loop over the byte array can be a foreach loop :
like this :
for (byte aResult : computeSha1Hash(input)) {
builder.append(Integer.toString((aResult & 0xff) + 0x100, 16).substring(1));
}- coupling should be reduced. The hashing algorithm (sha1), output mechanism (files) and byte[] to String encoding are all hardwired.
- use of magic numbers; replace with explaining constants.
Code Snippets
for (byte aResult : computeSha1Hash(input)) {
builder.append(Integer.toString((aResult & 0xff) + 0x100, 16).substring(1));
}Context
StackExchange Code Review Q#29501, answer score: 2
Revisions (0)
No revisions yet.