3
\$\begingroup\$

I'm a freshwoman at Systems Information, new to Java and programming and was assigned this task by my professor. My job is to read characters from a text file (file details in the bottom), recognizing each char as an integer. I should extend a class that will make bitwise operations to each char as to change them. For example, 'a' value is 97. Then it will be changed into something else, for example, 112, which is the number for 'p'. Then I have to print this modified character in a new file. I should send it by Thursday but I’m trying to make it cleaner and more efficient in the meanwhile. I’m working on handling “file not found” and “file can’t be read” with try/catch as we just learned it and maybe the professor would prefer that these exceptions would be handled that way and I’ll see if I can keep line breaks (-1).

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;

public class Codigo extends Codificador {

    @Override
    void codifica(String[] args) {

        File filein = new File(args[0]); // File to be read and encoded
        File fileout = new File(args[1]); // File that will receive encoded characters 

        FileInputStream fis = null;
        FileOutputStream fos = null;

        if (!filein.exists()) { // Tests if filein exists in this folder
            System.out.println(args[0] + " não encontrado."); // Returns message "File* not found" if needed.
            return;
        }
        if (!(filein.isFile() && filein.canRead())) { // Tests if filein can be read
            System.out.println(filein.getName() + " não pode ser lido.");  // Returns "File* can't be read" if needed.
            return; 
        }
        try {
            fis = new FileInputStream(filein);
            fos = new FileOutputStream(fileout);
            int c; // Character read from filein
            int r; // Character that will store encoded character c



            while ((c = fis.read()) != -1) {    // Each character from filein is read and stored in c
                                                // While the program doesn't fine a line break (-1)

                r = ((c & 0x0010) >> 4) | ((c & 0x0001) << 4) | // Bitwise operations are realized to encode c 
                    ((c & 0x1000) >> 3) | ((c & 0x0200) << 3) | // This encoded character is stored in r
                    (c & 0xEDEE);
            System.out.print((char)r); // Checks if was encoded as expected

            fos.write(r); // Writes encoded character in fileout
            }
        } catch (IOException e) {
            System.err.println("Caught IOException: " +  e.getMessage());

        } 
        finally {
            if (fis != null) {
                try {
                    fis.close();
                } catch (IOException e) {
                    e.printStackTrace();
                } // Closes FileInputStream resources
            }
            if (fos != null) {
                try {
                    fos.close();
                } catch (IOException e) {
                    e.printStackTrace();
                } // Closes FileOutputStream resources
            }
        } 
    }
}

This is the class that it extends:

public abstract class Codificador {
/** Recebe o nome do arquivo de entrada em args[0] e o nome do arquivo de
    saída em args[1]. Pressupõe que os arquivos estão em UTF-8. */
    abstract void codifica (String[] args);
/** codifica o caracter armazenado em c, troca os bits de lugar e
    retorna o valor.
    @param c: caracter a codificar
    @return : caracter em código
*/
    int codifica (int c) {
        int r;
        r = ((c & 0x0010) >> 4) | ((c & 0x0001) << 4) |
            ((c & 0x1000) >> 3) | ((c & 0x0200) << 3) | 
            (c & 0xEDEE); 
        return r;
    }
}

And here is a class to test it :

public class TestaCodigo {
    public static void main (String[] args) {
        Codigo cod = new Codigo ();
        cod.codifica (args);
    }
}

To test my code I should type

java TestaCodigo inputfile.txt outputfile.txt

Then, by running

java TestaCodigo outputfile.txt newfile.txt 

it should create newfile.txt which is a copy of inputfile.txt

These last ones were made by my professor, so I can't make any changes to them.

Any suggestions on code and/or documentation?

\$\endgroup\$
  • \$\begingroup\$ Instead of performing the bitwise operations in-place, call the method #codifica(int c) that is supposed to transform the character \$\endgroup\$ – WorldSEnder Jun 23 '15 at 17:32
  • \$\begingroup\$ @maaartinus, you can't add throws declarations when overriding a method from an abstract class afaik, thus you have to handle the IOException \$\endgroup\$ – WorldSEnder Jun 23 '15 at 17:33
  • \$\begingroup\$ @WorldSEnder Indeed, you can't throw more than prescribed by the overriden method. But 90% of time, throw new AnotherException(e) is the best handling you can do. System.err.println is the third worst (just after ignoring and using System.out). \$\endgroup\$ – maaartinus Jun 23 '15 at 17:52
  • \$\begingroup\$ Indeed, logging the excpetion is bad, I just meant that IOError is impossible. Propably should be IllegalArgumentException \$\endgroup\$ – WorldSEnder Jun 23 '15 at 18:00
2
\$\begingroup\$

I propose the following improvements (see below for some explaination):

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;

public class Codigo extends Codificador {

    @Override
    void codifica(String[] args) {
        File filein = new File(args[0]);
        File fileout = new File(args[1]);

        try (FileInputStream fis = new FileInputStream(filein);
                FileOutputStream fos = new FileOutputStream(fileout)) {
            int c; // Character read from filein
            while ((c = fis.read()) != -1) {
                fos.write(codifica(c));
            }
        } catch (IOException e) {
            throw new IllegalArgumentException(
                    "Can't or write from input/to output", e);
        }
    }
}

Don't explicitly check for files

If not explicitly stated by your professor, I wouldn't log anything to System.err. If you can't read from the input/can't write to the output file, this is a problem for the caller, not something you can deal with. A rule of thump is that problems related solemly to the arguments are to be dealt with by the caller because he has to decide how to deal with his illegal arguments.

As we can not simply rethrow the IOException because of the method declaration in Codificador (given be the prof, I assume), we catch them and throw an IllegalArgumentException with the IOException as its cause. If you are allowed to change the declaration in Codificador you should add an throws IOException and simply delete the catch-clause.

Using java's try-with-resource statement

Another improvement is to replace the try-finally with a try-with-resource clause. The way it internally works is that you can declare multiple resources after try (resource1; resource2; ....) which are automatically closed when you exit the try-block by calling their #close() method. This way, you don't have to deal with closing fis or fos manually, any longer, and they are always closed so you can't even implement the close errornously.

Using #codifica(int)

Generally you should avoid copying source. As the implementation of codifica is alread given in the base class, you should just call that method directly. This also allows child-classes to override the transformation.

\$\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.