9
\$\begingroup\$

I have a class that spawns threads to process data. I am working on instrumentation code. When the process is started, time = System.currentTimeMillis(); When it completes, time = System.currentTimeMillis() - time; I have a method to retrieve this time:

/**
 * Get the time taken for process to complete
 * 
 * @return Time this process has run if running; time it took to complete if not
 */
public long getRunTime() {
 return processRunning? System.currentTimeMillis() - time : time;
}

Is this a clear use of the ternary operator? Is my javadoc comment clear?

Zoran Jankov
7592 gold badges7 silver badges26 bronze badges
asked Feb 2, 2011 at 19:14
\$\endgroup\$
1
  • 3
    \$\begingroup\$ should document your units \$\endgroup\$ Commented Feb 3, 2011 at 3:03

4 Answers 4

6
\$\begingroup\$

Since the method has a return value even if the code is not finished processing the first line of your javadoc ought not read "Get the time taken for process to complete" but perhaps rather "Get the current or total processing time" or something of that nature? I also agree with Bert F's comment, but if there were clear comments explaining the behavior and you don't intend on exposing start and end times individually then there's no reason to waste an extra variable; you can either keep the code as it is and add comments or optionally get rid of the boolean "processRunning" and instead use your completedTime/runTime variable to establish the fact that it's still running. All minor gripes, all in all.

answered Feb 2, 2011 at 19:55
\$\endgroup\$
7
\$\begingroup\$

I have no problem the ternary operator.

But it is hard to tell that the result is what the comments is saying. Without having a context on what time is (which is a bad variable name) it is hard to understand the result of the function.

time: Bad variable name. Time of what?

answered Feb 2, 2011 at 20:33
\$\endgroup\$
5
\$\begingroup\$

It appears that the method sometimes returns a timestamp (time) and sometimes it returns a duration (difference between timestamps) (assuming that time is always a timestamp - if not, then this issue simply moves to time having a dual definition).

answered Feb 2, 2011 at 19:23
\$\endgroup\$
2
  • 3
    \$\begingroup\$ What about processRunning? System.currentTimeMillis() - startTime : completedTime - startTime; \$\endgroup\$ Commented Feb 2, 2011 at 19:26
  • 1
    \$\begingroup\$ Works for me, as does processRunning ? System.currentTimeMillis() - startTime : runTime; \$\endgroup\$ Commented Feb 2, 2011 at 19:30
2
\$\begingroup\$

It is a matter of style and taste but I'd rather go with

public long getRunTime() {
 if(processRunning)
 return System.currentTimeMillis() - time; 
 return time;
}

I just think this reads easier, and its easier to comment this. Now set me on fire for multiple return statements :D

answered Feb 21, 2011 at 13:22
\$\endgroup\$
2
  • \$\begingroup\$ Actually, I don't have a problem with this style. I do have a problem with multiple exit points spread throughout the function. \$\endgroup\$ Commented Feb 21, 2011 at 21:14
  • \$\begingroup\$ Looks a little odd without the else, IMHO. \$\endgroup\$ Commented Jan 22, 2012 at 8:03

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.