Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Dataflow anomaly analysis warnings from PMD

I am using Eclipse with the PMD Plug-in (4.0.0.v20130510-1000) and get a lot of those violations:

Found 'DD'-anomaly for variable 'freq' (lines '187'-'189').
Found 'DU'-anomaly for variable 'freq' (lines '189'-'333').

In this SO answer, it says that those anomalies are related to assigning values that are never read. But I get the violations for instance in this case:

// here I get a DD anomaly
double freq = 0;
try {
  // here I get a DU anomaly
  freq = Double.parseDouble(getFrequencyTextField().getText());
} catch (final NumberFormatException e) {
  Log.e(e.getMessage());
}
if (freq < 10E6) doSomething();

If I remove the initialization and add a freq = 0; line in the catch block, the DD anomaly vanishes, but I get a DU anomaly on both the assignments.

Now my question: How am I supposed to deal with that? What would be the preferred solution of PMD? And what exactly is this rule trying to prevent (i.e. why is it bad practice)?

like image 987
brimborium Avatar asked May 23 '13 15:05

brimborium


2 Answers

double freq; // (1)
try {
  // here I get a DU anomaly
  freq = Double.parseDouble(getFrequencyTextField().getText());
} catch (final NumberFormatException e) {
  Log.e(e.getMessage());
  freq = 0; // (2)
}
if (freq < 10E6) doSomething();

The first problem is that in the catch the parseDouble assignment is not done to freq. On an exception freq still would be 0. Maybe flaggable. So it goes away when assigning to freq inside catch.

When assigning to freq in the catch, (2), the inital assignment, (1), would never be read, so only a declaration suffices.

With respect to better style:

try {
  // here I get a DU anomaly
  double freq = Double.parseDouble(getFrequencyTextField().getText());

  if (freq < 10E6) doSomething();
  ...

} catch (final NumberFormatException e) {
  Log.e(e.getMessage());
}

Or follow the answer of @JoachimSauer, using a double conversion that does not throw an exception. The logging would indicate a level of seriousness in preference of the above style. Logging inside a simple conversion function on error might not be good style: too much logging, ignored logging (?), hard to repair.

like image 135
Joop Eggen Avatar answered Nov 04 '22 00:11

Joop Eggen


You could get around this problem (and separate concerns a bit more clearly) by extracting the parsing into a separate method:

double freq = parseDouble(getFrequencyTextField().getText());

// later in the class (or in a utility class):

public static double parseDouble(final String input) {
  try {
    return Double.parseDouble(input);
  } catch (final NumberFormatException e) {
    Log.e(e.getMessage());
    return 0;
  }
}

And if you have different default values, you could also add a two-argument version: public static double parseDouble(final String input, final double defaultValue).

like image 33
Joachim Sauer Avatar answered Nov 04 '22 00:11

Joachim Sauer