How to avoid repeating myself in nested loops

Question

I have a method with nested loops like below inside which I do some computationally expensive stuff and some computationally cheap stuff:

for(int i = 0; i < SIZE_I; ++i) {
    // Do cheap stuff 1
    // Do computationally expensive stuff 1
    for(int j = 0; j < SIZE_J; ++j) {
        // Do cheap stuff 2
        // Do computationally expensive stuff 2
        for(int k = 0; k < SIZE_K; ++k) {
            // Do cheap stuff 3
            // Do computationally expensive stuff 3
        }
    }
}

Currently, I call my method once. But I need to separate my cheap stuff from my expensive stuff. The problem is that if I develop two methods, I will need to repeat the nested loops and lots of code tangled with them.

I wonder if there is a best practice or tool to help me break my single method into two methods without repeating a whole lot of code. Or maybe if there is a solution to separate the cheap from the expensive without the need to break down my single method into two methods?

What? Could you please make a real example? I have two read twice your question to understand, I guess. — gsamaras, Nov 26 '18 at 12:54
@gsamaras I can develop a real example. My code is a bit lengthy, let me try ... — Megidd, Nov 26 '18 at 12:56
Why not just put each block of code into its own function? That's pretty much what functions are for. — Alan Birtles, Nov 26 '18 at 12:58
`std::function` to the rescue! If the airity is different then consider templates. Or consider living with the duplicate code. P.S. Are you sure `int` is an appropriate indexing type? — Bathsheba, Nov 26 '18 at 13:02
No one mentioned *cough* a bool argument to the function with *cough* a default value and some `if` around the expensive stuff... — YSC, Nov 26 '18 at 13:10
@Bathsheba Right. `int` is definitely not the best indexing type. I will take care of it later :) — Megidd, Nov 26 '18 at 13:11
@Bathsheba [Huh?](https://stackoverflow.com/a/24104825/4832499) Not to mention sometimes insane performance penalties. — Passer By, Nov 26 '18 at 13:15
@PasserBy: Which for me epitomises why you should never take anything you see on the internet at face value. What about platforms with 16 bit `int` and the ability to allocate larger arrays than 32767? — Bathsheba, Nov 26 '18 at 13:18
@PasserBy: ͏͏͏͏͏͏͏͏͏͏͏͏͏. Just use a `size_t` or a `ptrdiff_t` if negative values are defined. Pretty please, with sugar on top. — Bathsheba, Nov 26 '18 at 13:20
@Bathsheba Granted I don't work on embedded platforms, but I've been wrecked by unsigned craziness more than integer overflow. `ptrdiff_t` is fine. — Passer By, Nov 26 '18 at 13:24
@Bathsheba Do you often work with arrays of length >32767 but <65535? That extra bit is seldom useful, often harmful. Still waiting for `ptrdiff_t`. — YSC, Nov 26 '18 at 13:27
Do the various stuffs still need to be executed in order, or is it fine if you end up with twice the loops? — Quentin, Nov 26 '18 at 13:28
@PasserBy Your link is a perfect example why raising accepted answer to the top is evil. That accepted answer advocating `unsigned` is a nonsense. — llllllllll, Nov 26 '18 at 13:33
@Quentin Stuff labeled 1, 2 and 3 are in order and are tangled. The actual code is kind of complicated — Megidd, Nov 26 '18 at 14:41

score 1 · Accepted Answer · answered Nov 28 '18 at 11:15

I ended up doing this:

enum CallStatus {
    CallStatus_Cheap = 0,
    CallStatus_Expensive
};

bool MyClass::MyMethod(MyClass::CallStatus callStatus)
{
    for(int i = 0; i < SIZE_I; ++i) {

        switch (callStatus) {
        case MyClass::CallStatus_Cheap:
            // Do cheap stuff 1
            break;
        case MyClass::CallStatus_Expensive:
            // Do computationally expensive stuff 1
            break;
        default:
            break;
        }

        for(int j = 0; j < SIZE_J; ++j) {

            switch (callStatus) {
            case MyClass::CallStatus_Cheap:
                // Do cheap stuff 2
                break;
            case MyClass::CallStatus_Expensive:
                // Do computationally expensive stuff 2
                break;
            default:
                break;
            }

            for(int k = 0; k < SIZE_K; ++k) {

                switch (callStatus) {
                case MyClass::CallStatus_Cheap:
                    // Do cheap stuff 3
                    break;
                case MyClass::CallStatus_Expensive:
                    // Do computationally expensive stuff 3
                    break;
                default:
                    break;
                }

            }
        }
    }

// ...

}

Making use of enum as argument/parameter and switch, now I'm able to do cheap and expensive stuff separately, even though they are very entangled in nested loops.

BTW you can turn your `CallStatus` parameter into a template parameter if you discover that your `switch` statement is too expensive. — anatolyg, Nov 28 '18 at 12:25

score 0 · Answer 2 · answered Nov 26 '18 at 12:59

0

You can make a function which takes functions or lambda expressions as arguments. It would do the nested loops and apply "cheap stuff" or "expensive stuff" that you passed as parameter.

You would call this once for "cheap stuff" and once for "expensive stuff".

answered Nov 26 '18 at 12:59

Xaxetrov

54
4

1

I didn't occur to me to use lambda. I will give it a try – Megidd Nov 26 '18 at 13:02

How to avoid repeating myself in nested loops

2 Answers2