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

Reading from file, changing characters and writing to another file with FileInput/OutputStream

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

Problem

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) {

Solution

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.

Code Snippets

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);
        }
    }
}

Context

StackExchange Code Review Q#94141, answer score: 2

Revisions (0)

No revisions yet.