Unused private methods, private fields and local variables

Question

We are using Sonar to review our codebase. There are few violations for Unused private method, Unused private field and Unused local variable.

As per my understanding private methods and private fields can be accessed outside of the class only through reflection and Java Native Interface. We are not using JNI in our code base, but using reflection in some places.

So what we are planning is to do a complete workspace search for these methods and fields and if these are not used anywhere even through reflection, then these will be commented out. Again chances for accessing private methods and fields through reflection are very less. This is for safer side.

Unused local variables can’t be accessed outside of the method. So we can comment out these.

Do you have any other suggestions about this?

Answer 1

I love reflection myself, but to put it in a few words: it can be a nightmare. Keep java reflection to a very controlable (that is, stateless, no global/external variable usage) and minimal scope.

What to look for?

To find private fields and methods turned public, look for Field#setAccessible() and Method#setAccessible(), such as the examples below:

Field privateNameField = Person.class.getDeclaredField("name");
privateNameField.setAccessible(true);

Method privatePersonMethod = Person.class.getDeclaredMethod("personMeth", null);
privatePersonMethod.setAccessible(true);

So, setAccessible() will get you some smoke, but getDeclaredField() and getDeclaredMethod() are really where the fields are accessed (what really makes the fire).

Pay special attention to the values used in them, specially if they are variables (they probably will be), as they are what determine the field accessed.

Do a plain text search

Also, doing a plain text search for the field/method name on the whole project folder is very useful. I'd say, if you are not sure, don't delete before doing a full text search.

If you have many other projects that depend on this one you are trying to change; and if you weren't (or didn't know) the guy who planted those (bombs), I'd let it go. Only would change if really really needed to. The best action would be to get them one by one when you need to make a change to a code around it.

Ah, and, if you have them, running tests with code coverage can also help you big time in spotting unused code.