xeu
xeu

Reputation: 3

Is it better to add a conditional inside or outside of a for loop for cleaner code?

So this may sound simple, but I have a method that has a for loop inside, inside the forloop the method createprints needs a map of "parameters" which it gets from getParameters, now there are 2 types of reports, one has a general set of parameters and another has that general set and a set of its own.

I have two options:

Either have 2 getparameters method, one that is general and another that is for rp2 but also calls the general parameters method. If I do this then it would make sense to add the conditional before the for loop like this :

    theMethod(){

        if (rp1){
            for loop{
                createPrints(getgenParameters())
                do general forloop stuff 
           }
        }else{
              for loop{
                createPrints(getParameters())
                do general forloop stuff
                }
        }
    }

This way it only checks once which parameters method to call instead of having the if statement inside the loop so that it checks every iteration (this is bad because the report type will never change throughout the loop) but then this way, repeating the for loop looks ugly and not clean at all, is there a cleaner way to design this?

The other option is to pass in a boolean to the get parameters method and basically you check inside which type of report it is and based on that you create the map, this however adds a conditional at each iteration as well.

From a performance perspective, it makes sense to have the conditional outside the loop so that its not redundantly checked at each iteration, but it doesnt look clean, and my boss really cares about how clean code looks, he didnt like me using an if else code block instead of doing it with ternary operators since ternary only uses one line (i think performance is still the same no?).

Forgot to mention I am using java, i cannot assign functions to variables or use callbacks Inside the method, there was an if else code block before the for loop something like

String aVariable;

if(condition){
aVariable= value1;
}else{
aVariable =value2;
}

so i initially wanted to just create a boolean variable like isreport1 and inside if/else code block also assign the value cause it was using the same condition. And then as mentioned before pass in the parameter but, my boss again said not use booleans in parameters, so is this case the same I shouldnt do it here?

Upvotes: 0

Views: 552

Answers (3)

Stephen C
Stephen C

Reputation: 718906

Most "debates" about whether one way of writing some chunk of code is more efficient than another are missing the point.

The JIT compiler performs a lot of optimizations on your code behinds the scenes. It (or more precisely the people who wrote it) know a lot more about how to optimize than you do. And they will have the advantage of having done extensive benchmarking and (machine level) code walk-throughs and the like to figure out the best way to optimize.

The compiler can also optimize more reliably than a human can. And its developers are constantly improving it.

And the compiler does not need to be paid a salary to optimize code. Yes. Every hour you expend on optimizing is time that you could be spending on something else.

So the best strategy for you when optimizing is as follows:

  1. First get the code implemented. Time spent on optimizing while coding is probably wasted. That is classic "premature optimization" behavior. Your intuition is probably not good enough to make the right call at this stage. (Few people are genuinely smart enough ...)
  2. Next get the code working and passing its tests. Optimizing buggy code is missing the point.
  3. Set some realistic, quantifiable goals for performance. ("As fast as possible" is neither realistic or quantifiable). Your code just needs to be fast enough to meet the business requirements. If it is already fast enough, then you are wasting developer time by optimizing it further.
  4. Create a realistic benchmark so that you can measure your application's performance. If you can't (or don't) measure, you won't know if you have met your goals, or if a particular attempted optimization has improved things.
  5. Use profiling to determine which parts of your code are worth the effort optimizing. Profiling tells you the hotspots where most of the time is spent. Focus there ... not on stuff that is only executed occasionally. This is where the 80-20 rule comes in.

I would talk to my boss about this. If the boss is not on board with the idea of being methodical and scientific about performance / optimization, then don't fight it. Just do what the boss says. (But when be willing to push back if you are blamed for missed deadlines that are due to time wasted on unnecessary optimization.)

There are two kinds of efficiency in Software Engineering:

  • The efficiency of the program
  • The efficiency of the programmer

These two efficiencies are in conflict.

Upvotes: 1

maaartinus
maaartinus

Reputation: 46432

The branch prediction is a property of the CPU, not of the compiler. All modern CPUs have it, don't worry.

Most compilers can pull a constant condition out of the loop, so don't worry again. Java does it, too. Details: The javac compiler does not do it, it's job is to transform source cod to byte code and nothing else. But at runtime, the time-critical parts of the byte code get compiled in the machine and there many optimizations happen.

Forgot to mention I am using java, i cannot assign functions to variables or use callbacks.

You sort of can. It's implemented via anonymous classes and since Java 8, there are lambdas. Surely worth learning, but not relevant to your case.

I'd simply go for

       for loop{
            createPrints(rp1 ? getgenParameters() : getParameters())
            do general forloop stuff 
       }

as it's the shortest and cleanest way for doing it.

There are tons of alternatives like defining Parameters getSomeParameters(boolean rp1), which you can create easily using "extract method" from the above ternary.

From a performance perspective, it makes sense to have the conditional outside the loop so that its not redundantly checked at each iteration, but it doesn't look clean, and my boss really cares about how clean code looks

From a performance perspective, it all doesn't matter. The compiler is damn smart and knows tons of optimizations. Just write clean code with short methods, so it can do its job properly.

he didnt like me using an if else code block instead of doing it with ternary operators since ternary only uses one line

A simple ternary one-liner makes the code much easier to understand, so you should go for it (complicated ternaries may be hard to grasp, but that's not the case here).

(i think performance is still the same no?).

Definitely. Note that

  • most program parts are irrelevant for the performance (Pareto principle)
  • low level optimizations rarely matter (the compiler knows them better)
  • clean code using proper data structures matters

Upvotes: 1

T.S.
T.S.

Reputation: 19350

How about this ?

theMethod(getParametersCallback){

        for loop {
            createPrints(getgenParametersCallback)
            do general forloop stuff 
       }
}

. . .  . .

if (rp1){
    theMethod(getgenParameters)
}
else {
    theMethod(getParameters)
}

Upvotes: 0

Related Questions