5
\$\begingroup\$

I started learning java relatively recently as my first computer language independently and would like to see if my approach is efficient. An exercise in the book I am studying from tells me to read a file and write it into a seperate file with each line numbered.

Here is my solution

public class FileStuff
{
    public static void main (String [] args) 
    {
        System.out.println("Which file do you want to read");
        Scanner in = new Scanner(System.in);
        String fileInput = in.next();
        System.out.println("Which file do you want to write to?");
        String fileOutput = in.next();
        Scanner goThrough = null;
        PrintWriter print = null;
        String content ="";
        try
        {
            print = new PrintWriter(fileOutput);
            goThrough = new Scanner(new File(fileInput));
            while(goThrough.hasNextLine())
            {

                content = goThrough.nextLine();
                writeToLine(content, print);

            }
        } catch(FileNotFoundException e)
        {
            e.getMessage();
        }finally
        {
            goThrough.close();
            print.close();
        }

    }
static int lineNumber = 1;
    static void writeToLine(String line, PrintWriter out) throws FileNotFoundException
    {
        try
        {
        out.println(String.format("/* %d */ %s ",lineNumber,line));
        lineNumber+=1;
        }
        catch(Exception e)
        {
            e.getMessage();
        }
    }

}

Concerns

I have several inquiries about my code as I cannot understand my book clearly, and coding practicing questions.

Firstly, is it ok to initialize goThrough and print outside the try block as null? I did this because I wanted to close the Scanner and PrintWriter inside the finally block, but did not want to define something inside the try block since it is a different scope from the finally block

Secondly: Is it fine to throw FileNotFoundException at my method writeToLine? Or should I be throwing it after public static void main(String [] args), I have seen it done both ways in my book.

\$\endgroup\$
1
\$\begingroup\$

Here are my comments:

1) File handling

Your file handling logic is in the right direction:

  1. the File handlers (goThrough and print) are defined outside of the try block,
  2. are opened at the beginning inside the block
  3. are closed in the finally clause.

However, you forget to protect the code from NPE (NullPointerException): at the beginning inside the try block, both handlers contain null. You first open print the output handler, and then goThrough. However, in the finally clause, the order is reversed. So, if an exception is thrown while opening print, goThrough is never initialized and you might encounter NPE in the finally clause. it is absolutely essential to ask separately about each file handler before closing it:

if (goThrough != null) goThrough.close();
if (print != null) print.close();

Note: the best practice rule dictates that you first open the input handler, then the output one, and closes them in the reverse order.

As you can see, proper file handling is a delicate process that requires careful attention to details at different points in the code. Since many errors were made by many a good developers in many production programs, Java 7 added a feature that automatically handles this logic for you. it is called try-with-resources and it means that you declare and open the handlers at the point of the try block and the compiler adds the correct finally clause. I leave to you as study exercise to go though the tutorial and implement the feature into your code.

2) Exception handling

Exception handling in writeToLine() is incorrect:

  1. The method is declared with throws FileNotFoundException clause. However, the code is surrounded with try-catch that catches all exceptions, including the supposedly thrown one.

  2. The catch block contains what looks like a statement fragment that does nothing (but passes compilation). perhaps you meant to throw an Exception?

3) Design

This last comment falls into "best practice" category: the main() method does too many unrelated tasks: receive input from user, file handling and main loop. if, for example, you have a separate method that is responsible for communicating with the user, receiving user input and validating it, then perhaps you would be able to check for existence of input file as part of validation... then again, one method for receiving user input and validating it? hmm maybe we can design further break down ...

\$\endgroup\$
0
\$\begingroup\$

Your approach to define the variables is fine since you are trying to close using those variables in the finally block.

But throwing the FileNotFoundException in the writeToLine is actually not required as there are no file related operations other than the write. This exception is likely to be thrown when the file is opened while initializing the goThrough variable.

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