HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

File I/O, SHA1 sum and compression

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filecompressionsumandsha1

Problem

The function 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 :

  • 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 void as 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.