CODEWITHSUNDEEP
javaeclipse
F. Eser
F. Eser

Reputation: 155

Eclipse dead code warning, but what is wrong?

I've written a method that checks if my string is in a file, but eclipse gives dead code warning. Here is my method:

private boolean KeyPresent(String key){
    try{
        BufferedReader fileReader=new BufferedReader(new FileReader(keyPath));  
        while(true){
            String s=fileReader.readLine();
            if(s.equals(key)){
                fileReader.close();
                return true;
            }else if(s==null){
                fileReader.close();
                return false;
            }
        }
    }catch(IOException e){
            e.getStackTrace()
            return false;
    }
}

The else if(s==null) part is the source of the warning. Why? If you can't find any matching result (and coming outputs are all null), return false. I think it is OK. What is wrong?

And one more question. Which is better to use?

String s;
while(true){
    s="New Value";
    ....
}

or

while(true){
    String s="new value";
    ...
}

I think garbage collectors consume system resources, so the first one is better. However, I see more examples on the second one. Which one would you use?

Upvotes: 2

Views: 1485

Answers (3)

provisota
provisota

Reputation: 1660

Answerd is a simple: if s will by null, so in this row 

s.equals(key)

NullPointerException will be thrown, and programm will never rich this

else if (s == null)

condition. And programm will rich this condition only if s != null; so this condition always will be false. You better rewrite this snippet this way:

while (true) {
            String s = fileReader.readLine();
            if (s == null) {
                fileReader.close();
                return false;
            } else if (s.equals(key)) {
                fileReader.close();
                return true;
            }
        }

Upvotes: 1

Thiht
Thiht

Reputation: 282

If s is null, your first if will be executed and a NullPointerException will be thrown. Your else if can't be executed. You have to switch your if and else if to handle the null pointer properly.

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1499890

Look at the whole if/else:

if (s.equals(key)) {
    ...
} else if (s == null) {
    ...
}

If s is null, then s.equals(key) will have thrown a NullPointerException - so you'll never get into the second if block.

You should use a try-with-resources block anyway, and personally I wouldn't catch the IOException either... just let it bubble up:

private boolean keyPresent(String key) throws IOException {
    try (BufferedReader reader = new BufferedReader(new FileReader(keyPath))) {
        String line;
        while ((line = reader.readLine()) != null) {
            if (line.equals(key)) {
                return true;
            }
        }
        return false;
    }
}

Note that there's no garbage collection difference here; declaring the variable before the loop just means you can assign the value within the while condition.

Upvotes: 8

Related Questions