Read and convert binary files to ASCII text

Question

This is my code which receives a file to read and converts binary values to ASCII text.

The code works fine, so I am looking for a review on what I have written, and suggestions on how I could do it better, perhaps without the use of a StringBuffer.

import java.io.File;
import java.util.Scanner;

/**
 *
 * @author Tumpi
 */
public class DN6 {

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

    File file = new File("sporocilo.txt");

    Scanner sc = new Scanner(file);
    String lastString = "";

    while (sc.hasNextLine()) {
        String line = sc.nextLine();
        lastString = lastString + line;
    }
        StringBuffer result = new StringBuffer();
        for (int i = 0; i < lastString.length(); i += 8) {
            result.append((char) Integer.parseInt(lastString.substring(i, i + 8), 2));
        }
        System.out.println(result);
    } 

}

Answer 1

Aside from turning StringBuffer into StringBuilder, there are a few more things I'd like to note about your code.

Compound assignment operator

This line:

 lastString = lastString + line;

Can be rewritten as this one:

 lastString += line;

It's short and it complies with what many people would expect: a concatenation. By explicitly mentioning the original variable again, many people might think it is a different variable before looking at it more closely.

Additionally, you might also want to add a StringBuilder to the reading of the file. Even if the inputfile isn't that big, it would still be consistent.

Magic values

Consider this code:

for (int i = 0; i < lastString.length(); i += 8) {
     result.append((char) Integer.parseInt(lastString.substring(i, i + 8), 2));
}

Can someone other than the authro tell with certainty what the value 8 is? An educated guess would lead to the bit representation of 2 bytes, but we shouldn't have to guess that. Get rid of this so called "magic value" by adding a variable which clears it up:

int amountOfBits = 8;
for (int i = 0; i < lastString.length(); i += amountOfBits ) {
     result.append((char) Integer.parseInt(lastString.substring(i, i + amountOfBits ), 2));
}

Exception handling

You're completely bypassing exception handling. I'm assuming this is just so this contrived sample wouldn't get cluttered with code, but just in case it isn't:

throws Exception

is always a bad idea in an actual program. You should catch the exact exception and handle it appropriately.

Answer 2

You should be using StringBuilder instead of StringBuffer, since StringBuffer has synchronization for thread safety that you don't need.

To make the code more modular and reusable, consider writing it as a FilterInputStream instead.

Answer 3

In adition to everything else being said, I would like to point out that your tabbing is done incorrectly, and would look much sexier like this

import java.io.File;
import java.util.Scanner;

/**
 * @author Tumpi
 */
public class DN6 {
    public static void main(String[] args)
            throws Exception {

        File file = new File("sporocilo.txt");

        Scanner sc = new Scanner(file);
        String lastString = "";

        while (sc.hasNextLine()) {
            String line = sc.nextLine();
            lastString = lastString + line;
        }

        StringBuffer result = new StringBuffer();
        for (int i = 0; i < lastString.length(); i += 8) {
            result.append((char) Integer.parseInt(lastString.substring(i, i + 8), 2));
        }
        System.out.println(result);
    }
}

Answer 4

Because of the result String being of a size that can be pre-determined, and you are handling chars on the way, I would use a char[] (char array), and transforming the char array to a String using a String constructor.

I believe this is more efficient than using a StringBuilder or any other approach.

String lastString = "0100100001100101011011000110110001101111001000000101011101101111011100100110110001100100";
char[] result = new char[lastString.length() / 8];
for (int i = 0; i < lastString.length(); i += 8) {
    String sub = lastString.substring(i, i + 8);
    result[i / 8] = (char) Integer.parseInt(sub, 2);
}
System.out.println(new String(result));

You could also store the resulting string length in a variable, and switch the i variable to only iterate from 0 to the resulting string length.

String lastString = "0100100001100101011011000110110001101111001000000101011101101111011100100110110001100100";
int resultLength = lastString.length() / 8;
char[] result = new char[resultLength];
for (int i = 0; i < resultLength; i++) {
    String sub = lastString.substring(i * 8, (i + 1) * 8);
    result[i] = (char) Integer.parseInt(sub, 2);
}
System.out.println(new String(result));

Answer 5

• You absolutely must close that Scanner.

• This snippet

The scanner loop

while (sc.hasNextLine()) {
    String line = sc.nextLine();
    lastString = lastString + line;
}

creates a new String Object every time it loops. I also have no idea of how large you file is, but this will ultimately read the entire file into memory. In fact if you want to fully build lastString as you do here, you're better of using a StringBuilder.

• Can you avoid StringBuffer (and StringBuilder)? Well, certainly. You can handle the file character by character, and print everything to the System.out character by character as well :

example :

public static void main(String[] args) {

    File file = new File("sporocilo.txt");

    try (Reader reader = new InputStreamReader(new FileInputStream(file))) {
        int c = 0;
        int count = 0;
        for (int readChar = reader.read(); readChar != -1; readChar = reader.read()) {
            switch (readChar) {
                case '0':
                case '1':
                    c <<= 1;
                    c |= readChar - '0';
                    count++;
                    break;
                default:
            }
            if (count == 8) {
                System.out.print((char) c);
                c = 0;
                count = 0;
            }
        }
        System.out.println();
    } catch (IOException e) {
        e.printStackTrace();
    }
}