No return on method?

Asked

Viewed 1,382 times

6

I have a method where its function is to read a file and store the value written in the file in a variable and return the variable,:

public String addItemCreative(File f){
    String line = null;
    try{
        BufferedReader br = new BufferedReader(new FileReader(f));
        while( (line = br.readLine()) != null){
            line = br.readLine();
        }
    }catch(Exception e){
        e.getMessage();
    }
  return line;
}

The value returned is null because?

  • 1

    Puts a sysout within the while and see if it’s displayed.

  • Can you put the contents of the file in the question? Just click on [Edit].

5 answers

12


Read a File to a String

You don’t need to create a method for this. The recommended way to read an entire file to a String is the next:

String dados = new String(Files.readAllBytes(file.toPath()));

Preferably, specify a encoding to avoid problems with special characters:

String dados = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8);

Problems with some implementations

Concatenating Strings

One of the answers offers:

public static String addItemCreative(File f) {
    String line, lines = "";
    try {
        BufferedReader br = new BufferedReader(new FileReader(f));
        while ((line = br.readLine()) != null) {
            lines += line;
        }
    } catch (Exception e) {
        e.getMessage();
    }
    return lines;
}

This is not good because each concatenation of two Strings generates a third and memory is consumed exponentially. If the file is large, it will force the Garbage Collector to run several times, pausing the execution of the program and leading to very bad performance.

The right thing to do is to use StringBuilder in local implementations, which is a class that allows concatenating Strings without the need to synchronize the object. The @Cantoni response provides an example of how to do this.

Line breaks

If the file has line breaks, you are ignoring them by joining in the String. By calling the method readLine Java does not include breaks.

If you want the contents of the String to match the contents of the file, you must concatenate the breaks manually.

Calling readLine more than once

In the implementation of the question, the method readLine is called twice. If the file contains a single line, the second call will return null.

If the file has more lines, the result will be a String with the odd lines of the file.

Handling of exceptions

The exception that must be captured is IOException. Do not capture more generic exceptions unnecessarily. You may end up covering up mistakes.

Also, call the method getMessage does nothing. At least you should print the error log to understand what is happening. For example:

} catch(IOException e) {
    e.printStackTrace();
}

Or capture the mistake and do something with it

} catch(IOException e) {
    String erro = e.getMessage();
    mostraErro(erro);
}

Missing close the file

In Java it is easy to forget that not every resource allocated is automatically released.

When opening a file for reading or writing, remember to close the file later.

This can be done through the method close of one of the Reader used or automatically through try-with-resources of Java 6. Example:

try (BufferedReader br = new BufferedReader(new FileReader(f))) {
    ...
}

In the above code, the method close of BufferedReader will be automatically invoked at the end of the block try.

Good practice in general

Use variables in the smallest possible scope. Instead of declaring the variable line at the beginning of the method, it can be inside the block try and in the catch you return null directly. Reusing variables can be confusing.

A possible implementation bringing together everything I described above would be:

String quebra = System.getProperty("line.separator");

public String addItemCreative(File f) {
    try (BufferedReader br = new BufferedReader(new FileReader(f))) {
        StringBuilder sb = new StringBuilder();
        String line;
        while((line = br.readLine()) != null){
            sb.append(line);
            sb.append(quebra);
        }
        return sb.toString();
    } catch (IOException e) {
        e.printStackTrace();
        return null;
    }
}

Optional return

If you want to go further and semantically reinforce that the method has an optional return, for example in case the file does not exist or is empty, you can use the new Java 8 interfaces as Optional.

These interfaces avoid the need to use the null to specify no value. This avoids many NullPointerExceptions for lack of attention, since it forces the client code to verify if there is a value returned by the method.

Example:

String quebra = System.getProperty("line.separator");

public Optional<String> addItemCreative(File f) {
    try (BufferedReader br = new BufferedReader(new FileReader(f))) {
        StringBuilder sb = new StringBuilder();
        String line;
        while((line = br.readLine()) != null){
            sb.append(line);
            sb.append(quebra);
        }
        return sb.length() > 0 ? Optional.of(sb.toString()) : Optional.empty();
    } catch (IOException e) {
        e.printStackTrace();
        return Optional.empty();
    }
}

Now it is easy who calls the method to know that it may not return some value.

Assuming you wanted to print the content. The method could be used like this:

Optional<String> conteudo = addItemCreative(arquivo);
if (conteudo.isPresent()) {
    imprimir(conteudo.get());
} else {
    imprimir("[arquivo vazio]");
}

Still, if you prefer a more functional format:

imprimir(addItemCreative(arquivo).orElse("[arquivo vazio]"));
  • 2

    This should be the correct answer, as it is the most complete and contains very useful information for reading text files with Java. I hope the author reevaluate, because leaving the current answer as the correct one is not good for Sopt.

  • 1

    Vlw Uiz I helped mt avoid several errors, I will use your example.

  • 1

    With answers like these that makes Sopt be a huge didactic source amid so many tutorial sites.

  • But that maddening this one of Optional<?>, like.

  • 1

    @These classes that enable Functional Programming in Java is one of the coolest gambiarras ever invented. : D

7

From what I understand, the intent of your code is to read the contents of the entire file, as text, and return it, is that right? If it is, I have three observations:

  1. Start with the empty string, as if the file is empty it will return "", nay null;
  2. Use a variable different to store the return value and the line being read;
  3. When reading something and testing by null, don’t read again, because if you do this you will be reading the line next which was read, and not the same line. And worse, this new line may come null...

Example:

public String addItemCreative(File f) throws IOException {
    String ret = "";
    BufferedReader br = new BufferedReader(new FileReader(f));
    while( (line = br.readLine()) != null){
        ret = line;
    }
    return ret;
}

(Note: the example of Cantoni with StringBuilder is better than mine, because it avoids creating new objects String needlessly)

4

The fact that you are doing a silent treatment of exception is hurting you to understand what is happening. Exceptions can be handled silently, but as long as they do not prejudice later executions of the code.

Remove the exception treatment and see what is happening when you do:

BufferedReader br = new BufferedReader(new FileReader(f));

Your error happens on this line. Probably the file passed as parameter to Filereader does not exist.

UPDATING

A way to iterate over a text file can be seen below:

public String addItemCreative(File f){
    StringBuilder lines = new StringBuilder();
    String line;

    BufferedReader br = new BufferedReader(new FileReader(f));

    while( (line = br.readLine()) != null){
        lines.append(line);
    }

    return lines.toString();
}
  • There is, the problem was in the assignment to variable line

1

No way to know exactly the reason without you showing the file.

But it is very likely that the file does not have any lines, causing the code within the while (which is the part that gives a value to line) not be executed.

At the end of it all is returned line was assigned as null and received no value.

There may also be some error while trying to open the file and you are not seeing it as you are silencing the exception. If you take this try-catch you will see what is happening. It may be that the file does not exist, that you are not allowed to open it, etc.

  • Actually the file does not have a line, but it has characters. I will try using more than one line

0

Try to do as follows by concatenating line by line:

public static String addItemCreative(File f) {
    String line, lines = "";
    try {
        BufferedReader br = new BufferedReader(new FileReader(f));
        while ((line = br.readLine()) != null) {
            lines += line;
        }
    } catch (Exception e) {
        e.getMessage();
    }
    return lines;
}

Or also do as follows with Stringbuilder:

public String addItemCreative(File f) {

    StringBuilder lines = new StringBuilder();
    String line;
    try {
        BufferedReader br = new BufferedReader(new FileReader(f));
        while ((line = br.readLine()) != null) {
            lines.append(line);
        }
    } catch (Exception e) {
        e.getMessage();
    }
    return lines.toString();

}
  • Vlw, it worked perfectly

  • 1

    One thing that should be clear in this code and in the question code is that two line readings are made per iteration. A reading is done at the while and the other is done inside the while. Very likely this can cause problems, because the natural is to read a line per iteration.

  • Then using this code snippet should not give any error:

  • while(line+=br.readline()){ }

  • @Carloshenrique That’s not going to work either, it’s going into an infinite loop because line will never be empty, and he’ll just keep adding "null" at the end of the string a lifetime.

  • I didn’t put the != null pq this is logical

  • @Carloshenrique, I updated my answer with a way to read the file. The Bufferedreader readline returns the current line and moves on to the next one. Therefore, if you call it more than once inside the loop, then you will read more lines than you want.

  • Concatenate Strings in this way can easily lead to memory usage spikes and pauses for collecting Strings not used in memory. Correct is to use StringBuilder to concatenate the file and return a String at the end.

Show 3 more comments

Browser other questions tagged

You are not signed in. Login or sign up in order to post.