2
\$\begingroup\$

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 ?

\$\endgroup\$
  • \$\begingroup\$ If the problem is that you have to create a new instance of this class every time you want to use it in another class, it is possible that we'd refactor both classes. It might be better to post the class using this as well. As is, we don't even have a usage example. You also assign a value to delayOldValue but you never declare or use it. \$\endgroup\$ – mdfst13 Aug 31 '16 at 12:34
  • 1
    \$\begingroup\$ What is 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\$ – I'll add comments tomorrow Aug 31 '16 at 23:08
  • \$\begingroup\$ There was just a capitalized letter that shouldn't be that way ! \$\endgroup\$ – Hassam Abdelillah Sep 1 '16 at 6:29
  • \$\begingroup\$ You may want to consider using BigDecimal, instead of double - stackoverflow.com/questions/3413448/double-vs-bigdecimal \$\endgroup\$ – matt freake Sep 1 '16 at 13:45
3
\$\begingroup\$

Javadoc, Javadoc, Javadoc


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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.