1

A lot of questions popped up while I was looking at my code. I would like to know what people think about some code snippets. For each one, I am wondering what is more optimized, and what is better practice or cleaner. Most likely, any possible optimization is on the scale of nanoseconds, I am just wondering theoretically. :)

1)The first one is a frequently called routine that needs to report an error. Right now it looks like this

if(success)
 //Stuff
else
 reportInternalError();

Is any benefit to changing it to

if(success) {
 //Stuff
 return;
}
reportInternalError();

2)reportInternalError() is supposed to only report the error the first time it happens, and it can potentially be called very frequently. Currently, it is like this

if(!reported) {
 //Report error
 reported = true;
}

I am thinking about changing it to

if(reported)
 return;
//Report error
reported = true;

3)The last one is about duplicated code. I am using the exact same code twice, and I am trying to figure out a better way of writing it.

try {
 //Stuff
} catch(){
}

while(loop) {
 try {
     //Same Stuff
 } catch(){
     loop = false;
 }
}

Versus

first = true;
do {
 try {
     //Stuff
 } catch() {
     if(!first)
         loop = false;
 }
 first = false;
} while(loop);

Thanks!

Improve this question
1
  • 1
    Correct, any difference in runtime performance will be so incredibly negible nobody would ever notice. You shouldn't even think about it, even theoretically, as it's an utter waste of time ;) Commented Jan 29, 2011 at 12:25

6 Answers 6

Reset to default
4

Don't worry about it. At this day and age, computer power is a lot cheaper than brain power, so optimize your code for readability, not for maybe gaining a few microseconds here and there.

Besides, the JVM's JIT compiler will most likely do these optimizations for you. And even if it doesn't, it has become so complex that modifications that look like performance gains, may actually slow you program down. Josh Bloch gave an excellent presentation on this subject at Devoxx last year.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

One of my programming teachers had a good saying regarding this: "Don't try to outsmart the compiler"
2

I would put a premium on readability and maintainability over micro-optimizations. The gains from micro-optimization are really negligent (and often nil given the sophistication of today's compilers, JITs, and processors).

My choices:

1) I try to put the normal case along the main code path and the exception cases in the branches:

if (! success) {
    reportInternalError();
}

// Stuff

2) I favor the original approach. The other way assumes the method will always to just want to return, but what if it needs to do more stuff in the future? I don't want that extra stuff to go in the branch:

if (! reported) {
    // Report error
    reported = true;
}
// extra stuff to do

3) The refactored version seems more maintainable to me. I'm on the fence about replacing targetExceptionWasThrown with a simple break:

boolean isFirstIteraton = true;
boolean targetExceptionWasThrown = false;
do {
    try {
        //Stuff
    } catch() {
        if (! isFirstIteraton) {
            targetExceptionWasThrown = true;
        }
    }
    isFirstIteraton = false;
} while (! targetExceptionWasThrown);
Improve this answer

2 Comments

Thanks, you gave a lot of justification for your choices.
@Kevin S - thanks. I think the rationale is important because: (1) I could have faulty logic and want some to correct me. (2) There is no absolute one right way to do things that covers all cases - you need to know the rationale to know when to deviate.
2

Regarding #3, exception will 'exit' the loop for you, just put try-catch outside the loop.

try {
 //Stuff
} catch(){
}

 try {
   while(true) {
     //Same Stuff
   }
 } catch(){
 }

But since you have some amount of duplication, employing break in the second example might be better (instead of loop var).

first = true;
while (true) {
 try {
     //Stuff
 } catch() {
     if(!first)
         break;
 }
 first = false;
};

Otherwise, you could create a separate method for //Stuff. It would save clarity of the first example, but remove duplication.

Improve this answer

Comments

1

unless you are writing code for very low level (i.e. to a special microprocessor in assembly language), such improvements would only have (if any) tiny effect on performance. I agree that the readability is more crucial in high level.

Improve this answer

1 Comment

Thanks for the advice but this is something I already know and have mentioned in the OP. I was really looking for concrete answers.
1

Both theoretically and practically, your compiler should handle this kind of micro-optimization, not you. Focus on clarity instead and stick to the single-return idiom: every function/method should have one return statement, at the end.

In the third example, your intention seems to be to execute the try/catch block at least twice. Try

for (int i=0; i<2 || loop; i++) {
    // put the try/catch block here
}
Improve this answer

Comments

1

I agree with the other answers that you shouldn't try to outsmart the compiler for performance and instead focus on readability. What I use when i have the the case 1 and 2 is this:

case 1:

if( ! success ) {
  reportInternalError();
  return;
}

// do Stuff

and for case 2:

if(reported)
 return;

//Report error
reported = true;

Basically is the guard clause pattern. http://www.c2.com/cgi/wiki?GuardClause. This works properly when you want to make sure some invariant is met before running your code. I think is ok to break the no multiple returns rule in this case. And it make my code easier to read because i can check at a glance what conditions should be met before running the code.

Regarding #3 i would write like this:

    try {

        boolean finished = false;
        do {

           //Same Stuff

           if ( /* end of loop test */ ) {
               finished = true;
           }
        } while ( ! finished );
   } catch() {
      // handle exception       
   }
Improve this answer

Comments

Your Answer

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