I calculate the moving averages of a specific measure. However, multiples threads can access this class in order to compute the average.
public class AvgUtils {
/** The alpha. */
private static final double ALPHA = 0.9;
private double avgOldValue = 1.0;
public synchronized double computeAvg(double pValue) {
if (avgOldValue == 0.0d) {
avgOldValue = pValue;
return pValue;
}
double lValue = avgOldValue + ALPHA * (pValue - avgOldValue);
avgOldValue = lValue;
return lValue;
}
}
In the other class, I'm obliged to create a new instance of this class in other every time to do the calculations.
Can my class be refactored or redesigned in a better way ?
1 Answer 1
Your variable names are all sub-par. Why the unnecessary abbreviations (f.e. Avg
)? Why the odd prefixes (f.e. lValue
)?
Here is my simple and ultimate tip when it comes to names:
Name things after what they are doing or what they contain.
And my ultimate rule:
You're never allowed to use one letter variable names, with the only exception being dimenstions.
/** The alpha. */
private static final double ALPHA = 0.9;
That is a prime example of an unhelpful comment.
Overall, I don't see why your class is static. It seems like it will be used like an instance class but you don't want to pass the reference around, you simply created a singleton here.
Consider creating an instance class and then pass the reference/instance to the threads which share that instance.
delayOldValue
but you never declare or use it. \$\endgroup\$AvgOldValue
now? (no, it's not defined in your code) Please don't apply sloppy "hot fix" edits to the posted code that only introduce more errors. Post the real code you are using. Don't create some special version you derived to post here. Give us the real thing. This way, you get a real review. \$\endgroup\$