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 ?

asked Aug 31, 2016 at 12:15
\$\endgroup\$
4
  • \$\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\$ Commented Aug 31, 2016 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\$ Commented Aug 31, 2016 at 23:08
  • \$\begingroup\$ There was just a capitalized letter that shouldn't be that way ! \$\endgroup\$ Commented Sep 1, 2016 at 6:29
  • \$\begingroup\$ You may want to consider using BigDecimal, instead of double - stackoverflow.com/questions/3413448/double-vs-bigdecimal \$\endgroup\$ Commented Sep 1, 2016 at 13:45

1 Answer 1

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.

answered Aug 31, 2016 at 20:43
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.