Tried a for & while loop...what am I doing wrong?

[Brian845]

Excuse me, for I'm very new to c++. This is my first language I'm learning. Self taught with a udemy course, and web searches. I'm trying something on my own and could use some guidance. I know this is a beginners forum and my question seems so simple compared to other posts in here. I'm not as experienced as most of the other beginners, sorry!
Anyway, I have tried so many things with different loops to get the desired result. As you will see in the code presented below, I'm trying to get the sum of a row in a 2d vector to increment the row with ++b. I have named the sum lay_sum. And I'm incrementing a counter also by one. I just don't understand why the lay_sum is not RE-calculating corectly when "b" is in the calculation. When I run it through the debugger to try and find the problem, "b" gets incremented but the calculation for lay_sum doesn't. I'm not looking for someone to do the work for me, but I also would like an explanation so I can learn.

    int layer_rep_start {1};
    int a {0};
    int line_number  {1};
    int draws_reporting {4}
    
    while ( line_number <= draws_reporting) {
        int b {layer_rep_start};
        int sum_counter {0};
        int draw_sum {d5[a][0] + d5[a][1] + d5[a][2] + d5[a][3] + d5[a][4]};
        int lay_sum {d5[b][0] + d5[b][1] + d5[b][2] + d5[b][3] + d5[b][4]};
        //  The line above this comment does not recalculate when b increases.  WHY??
        
        while ( draw_sum != lay_sum ) {
            ++sum_counter;
            ++b;   
        }
        std::cout << line_number << "   " << sum_counter << std::endl;
        layer_rep_start++;
        line_number++;
        a++;
    }

[jonnin]

thin down the code and you can see the problem.

while(something)
  {
     b = initial value;
      while(other)
       {
         change b
        }
      //b is now changed
       do stuff not related to b at all
   } //loop back to... b is initial value line ... !!! 

also you are mixing and matching syntax a little oddly.
modern initializers are like this:
int x{42};
older code says
int x = 42;
you somehow combined these ideas. Prefer to use the no equals {} version.

declaring all these variables inside your loops may or may not be a good idea. There are times when it should be done, and times when its better to have them outside the loop. I prefer them outside unless they need to be inside for some purpose (like reinitialized on purpose, instead of by accident).
.at is slower than [] to access a vector, but .at is safer. prefer [] unless you plan to handle what happens if .at goes out of bounds. if[] goes out of bounds ... well, lets not do that ;)

don't worry about asking anything at all here, no matter how simple. Your question is perfect: you have code, its in code tags, you told us what was wrong with it. If you do that, you will get help when you need it here.

[Brian845]

also you are mixing and matching syntax a little oddly.
modern initializers are like this:
int x{42};
older code says
int x = 42;
you somehow combined these ideas.

OMG! I didn't even realize this. I'm laughing at myself right now, sorry.

I will change the .at method to the [ ]. Just wasn't real sure about the safety of going out of bounds. It should show in the IDE after the build.

Still not sure of the b thing yet. The reason I set it to an initial value was because it will also be used again in another totally different loop, that will be reset back to initial value.

EDIT:: Thanks for the kind words about my question. I must admit, I was a little hesitant for asking such an inexperienced question. At first post the code tags weren't working, had to edit a few times till it worked. Thanks again!

[JLBorges]

> Just wasn't real sure about the safety of going out of bounds.

For simple iteration, favour range-based loops over classical for loops. They are both safer and also more readable. https://www.stroustrup.com/C++11FAQ.html#for

For instance:

#include <iostream>
#include <vector>
#include <numeric>

int main()
{
    const std::vector< std::vector<int> > data_set { {9,4}, {0,1,2,3}, {7,8}, {4,5,6,7} };

    std::vector<int> row_sums ;
    for( const auto& row : data_set ) row_sums.push_back( std::accumulate( std::begin(row), std::end(row), 0 ) ) ;

    for( int s : row_sums ) std::cout << s << ' ' ;
    std::cout << '\n' ;
}

http://coliru.stacked-crooked.com/a/0dbdef4f0cef190f

[Brian845]

JL, Thanks for the reply!

Hmmmm... I got more learning to do.

[jonnin]

Still not sure of the b thing yet. The reason I set it to an initial value was because it will also be used again in another totally different loop, that will be reset back to initial value.

Maybe you need to stop to consider the logic, then.
I can tell you WHY
// The line above this comment does not recalculate when b increases. WHY??
the why is that you reset it, do stuff, then increment, then loop around and reset.. so in effect it never changes. But I am not sure what you want to change the logic to do differently.

Where is the loop that needs b to be reset? Its perfectly acceptable to do this

loop, modifying b
end loop
b = initial value again
loop, modifying b ...

though often such loops over the same things can be combined to avoid looping over the same data twice. Sometimes not possible. Worry about that later if you need to do so.

Listen to JL. He is far better than I am at writing modern, clean code. My 2 cents on using range based is the only time to NOT use them is if you need the loop variable for some purpose -- sometimes the counter is part of the algorithm, esp in cute 'contest' or 'interview' type questions that exist in a vacuum, and sometimes in mathematical coding.

[Brian845]

I think if I explain what I'm trying to accomplish a little better, it may be a little clearer.

I'm comparing rows to each other in a 2d vector. The first loop I'm comparing the sums. I get the sum for row "a" a ={d5[a]} row#1 and compairing that sum to row "b" b ={d5[b]} row#2, a=0: b=1;. If its not Equal, then ++b so the sum is now row#3 being compared to the sum from row "a". In a sense its like: sum of row1 compared to sum of row 2, if not equal, row1 compared to row3, if !=, 1 to 4..1 to 5...1 to 6......ect, until I find a match. There is a counter that increments after each row increment. I'm interested in the final result of the counter.

The first while loop is for counter outputs for line #1 comparisons. (Line no == row) in 2d vector basically.
The second while loop is for "sum" comparison for row1.
A third while loop will be for another comparison to row1(not "sum", but "somethng" else.
And another comparison to row1.

And a print out:
line no. sum counter another counter another counter.....etc.

Then increment "a", increment line no. increment layer_rep_start
Then go back up to the first while loop and do comparisons for line #2. (next row in vector).

And as you see the initial "b" = layer_rep_start That resets "b" so going through another set of all iterations....."a" now = 1 and "b" now =2......and go through the sets of while loops again for another output line of counters.

I'm always checking "a" against "b" and keep ++b until I find a match.


Hope I explained things better. Wow, I almost confused myself!!

[JLBorges]

Something like this, perhaps:

#include <iostream>
#include <vector>
#include <numeric>

int main()
{
    const std::vector< std::vector<int> > vec2d { {9,4,2}, {0,1,2,3}, {7,8}, {4,5,6,7} };

    // compute the sum of row zero
    const long long sum0 = std::accumulate( std::begin( vec2d[0] ), std::end( vec2d[0] ), 0LL ) ;

    // compare that sum with sum of row 1, row 2 etc. till we get a match
    std::size_t row = 1 ;
    for( ; row < vec2d.size() ; ++row )
    {
        // break out of the loop if the row sum is a match
        if( std::accumulate( std::begin( vec2d[row] ), std::end( vec2d[row] ), 0LL ) == sum0 ) break ;
    }

    if( row == vec2d.size() ) std::cout << "no row sum matching sum of row zero was found\n" ;
    else std::cout << "sum of row zero == sum of row " << row << " == " << sum0 << '\n' ;
}

http://coliru.stacked-crooked.com/a/a18c4f82f8d42a4f

[Brian845]

Hey JL thanks!!!! That looks very promising. There is a little bit of that code I got to do some research on. Not quite clear on some of it. I'm learning though!! Thanks again.

Now, I did get my code to run with desired/correct output. It may look pretty UGLY to you, but I got it to work. Definitely not optimized for speed!! I know it needs work, but I guess I'm happy taking baby steps first. I just thought about getting code to work first, then improving it when I get more knowledgeable about C++ and programming in general. Here is the code, let me know what you think:

    int layer_rep_start {1};
    int a {0};
    int line_number {1};
    
    while ( line_number <= draws_reporting) {
        int b {layer_rep_start};
        int sum_counter {0};
        int draw_sum {d5[a][0] + d5[a][1] + d5[a][2] + d5[a][3] + d5[a][4]};
        while (true) {
            int lay_sum {d5[b][0] + d5[b][1] + d5[b][2] + d5[b][3] + d5[b][4]};
            if ( lay_sum != draw_sum) {
                ++sum_counter;
                ++b;
            } else
                 break;
        }
    std::cout << line_number << "   " << sum_counter << std::endl;
    layer_rep_start++;
    line_number++;
    a++;
    }
        
    return 0;    
}

I'm a mess, aren't I??
There are a few more lines above this code, but not included to keep it short.

[jonnin]

producing working code is the first step. Code that does not work has no value no matter how pretty or modern or clever it is.
Speed isnt important until you tackle big problems. Modern desktop/laptop computers can do about 200 billion floating point (or lesser) operations per second. Until you have millions of pieces of data each taking many operations each to process, it is more important to focus on clean code. I worked for a long time using slow machines with real time requirements -- sometimes its hard to let go, but for the most part trying to go faster is not even a goal for most code.

[Brian845]

THANKS jonnin!!! It is motivating hearing nice things. Yeah, I was reallly happy with working code. This does get real frustrating when you are learning and have no personal friends to ask for help. This board seems like the place to go for help as long as you ask properly and try to give as much details as possible. I will not hesitate to ask a question again.

Thanks again,
Brian

[seeplus]

Producing a valid program design (including data structure(s), input/output requirements, processing algorithms etc) is the first step. Then code from the design then test/debug.

[jonnin]

True ^^. Schools undervalue that, and students as well. The schools gloss over it and the students THINK they lack the time not understanding yet that it SAVES time.

[seeplus]

When I was taught programming more years ago than I care to remember by a Professor of Computer Programming, we had to first produce a documented program design. That was marked before we started to code the program. Only once the design had 'passed' did we code. The design was 50% marks, the program was 40% and the final 10% was for testing evidence, program critique etc.