sum of k adjacent elements in array

[airscuba]

Hi to all.
This is my first topic.

My problem is this: when I run the code posted, the value of MAX at the end of "for" cicle is right, but when I write it in to the output.txt file the value is incorrect.

The compilation is right and not return error.

I think is a problem of out of range of array index, but I don't know how to solve.
Please help me. Thanks

#include <iostream>
#include <fstream> 
using namespace std; 
  
int main() 
    { 
     int N=0, k=0, i =0, j=0;
     int sum=0, MAX=0;

ifstream in("input.txt");   
fstream out("output.txt"); 

in >> N >> k; // Read N and k from first line of input.txt

int S1[N]; // initialize array dim

for(int i=0; i<N; i++){
    in >> S1[i]; // write a vector elements from second line of input.txt
//    out << S1[i];

   	}
// start to calculate sum of k elements   	
for (i=0; i<N; i++) {
	for (j=0+i; j<k+i; j++) {
		sum += S1[j];
	}
	if (sum > MAX) {
		 MAX = sum;
		if (j==N) {
		out << MAX << endl;
		}
	}
	somma = 0;
}

//out <<  MAX << " "; 
        return 0; 
    } 

This is an example of values in the input.txt
7 3
10 -3 -1 6 4 1 -10

and the result of MAX is 11 (to write in the output.txt)

[MikeStgt]

The compilation is right and not return error.

I disagree

 In function 'int main()':
33:2: error: 'somma' was not declared in this scope

Then,

the value of MAX at the end of "for" cicle is right

at the end of the "for() cycle" (aka loop) MAX is still 0. You set it outside the loop in a way that looks to me like a miscarried "prevent overflow" filter. If this was your intention you should set MAX to the maximum int may represent and use it before the sum += S1[j]; step. Execute this step only if sum < MAX - S1[j], otherwise... it's up to you how to handle this situation. (I just see no reason for MAX as sum would be sufficient.)

[lastchance]

This line is illegal in strict C++, because N is not a compile-time constant:
int S1[N];

You will certainly fall foul of out-of-bounds errors:
i can be as big as N-1 (that's OK).
j can then be as big as i + k - 1, or N+k-2 (that's NOT OK - the array isn't that big).

I presume you are muddling sum and somma up (so, as @MikeStgt says, your code won't compile). As a result, sum isn't being reset to 0 for each pass of the i loop.

Actually, initialising MAX to 0 is not necessarily good enough if your array contains all negative elements. (I presume MAX is meant to be the biggest value of sum for any i: wouldn't variable name max_sum convey that more obviously?).

Personally, since there is no feasible initial value of MAX, I would set its value for the first time when you have computed the sum for the i=0 loop, and test for changes thereafter; however, that's a personal preference. Others might initialise it to the minimum representable integer, but initialising to 0 is wrong here.

Please sort out your indentation and remove unused debugging lines when posting online.

[airscuba]

Thanks MikeStgt, Thanks lastchange.

Unfortunately I'm really a beginner ... Sorry.

I tried to make some changes following your suggestions but I could not solve.

Certainly I was not clear in explaining the problem and also the code presented some errors ... now I put it in the last version (also the indentation improved).

#include <iostream>
#include <fstream> 
using namespace std; 
  
int main() 
    { 
     int N=7, k=3, i =0, j=0;
     int sum, MaxSum;

     int S1[N] = {10, -3, -1, 6, 4, 1, -10}; 


     for (i=0; i<N; i++) {
	cout << "i = " << i << endl;
	MaxSum = 0;
	for (j=0+i; j<i+k; j++) {
		cout << " " << "k = " << k+i << " ";
		cout << " " << "j = " << j << " ";
		sum += S1[j];
		cout << " " << "sum = " << sum << " " << endl;
		if (sum > MaxSum) {
			MaxSum = sum;
			if (j==N-1) {
				cout << "MaxSum = " << MaxSum << endl;
			}
		}
	
	}
	sum = 0;
        }
       return 0; 
} 

Therefore, I must find the maximum sum between "k" adjacent elements of a vector of "N" elements "i"

In the example, the vector has N = 7, k = 3 and the elements vector are {10, -3, -1, 6, 4, 1, -10}.
The result should be 6 + 4 + 1 = 11 with i = 3 and k = 6

If I initialize the vector with the elements (as in the code posted), I get this incorrect result

i = 0
 k = 3  j = 0  sum = 10 
 k = 3  j = 1  sum = 7 
 k = 3  j = 2  sum = 6 
i = 1
 k = 4  j = 1  sum = -3 
 k = 4  j = 2  sum = -4 
 k = 4  j = 3  sum = 2 
i = 2
 k = 5  j = 2  sum = -1 
 k = 5  j = 3  sum = 5 
 k = 5  j = 4  sum = 9 
i = 3
 k = 6  j = 3  sum = 6 
 k = 6  j = 4  sum = 10 
 k = 6  j = 5  sum = 11 
i = 4
 k = 7  j = 4  sum = 4 
 k = 7  j = 5  sum = 5 
 k = 7  j = 6  sum = -5 
i = 5
 k = 8  j = 5  sum = 1 
 k = 8  j = 6  sum = -9 
 k = 8  j = 7  sum = 22038 
i = 6
 k = 9  j = 6  sum = -10 
 k = 9  j = 7  sum = 22037 
 k = 9  j = 8  sum = 44074 

------------------
(program exited with code: 0)
Press return to continue

I have tried various ways to block the count of k when I am at the end of the carrier but nothing to do.

I also thought of calculating the possible number of possible triplettes but I could not implement them.

I guess it's really a simple thing, but I've been trying for two days and I've lost hope of succeeding.

Thanks a lot for your help.

[lastchance]

On line 16 you are allowing j to go as high as one less than i + k. Thus, i cannot be allowed to go as high as one less than N. You must reduce the upper limit for i. Change your line 13 to
for (i=0; i<N-k+1; i++) {


sum is not initialised on the first i loop. You should set
sum = 0;
immediately after your current line 13; i.e. at the start of the i loop. This is much safer than where you have it at the moment on line 29.


MaxSum should only be initialised ONCE; you are currently re-setting it to zero on every pass though an i loop. Remove your current line 15 and replace your lines 21-26 by
if ( i == 0 || sum > MaxSum ) MaxSum = sum;
This will both initialise MaxSum on the first i loop and update it on subsequent loops if necessary.


You should just output MaxSum after all your loops have finished, not in response to j being N-1.


Finally, your declaration of the array on line 10 is still strictly illegal in standard C++, because N is a variable, not a constant. Your compiler may let you get away with it, because it is allowed in the latest version of C; however, don't rely on that.


Your indentation is still rather inconsistent and disorientating. You may find it easier to use spaces rather than tabs. However, well done for putting in some debugging lines.


If you make the changes above to your code I get MaxSum = 11.

[airscuba]

GREAT!

Thank you very much.

This is the clean and working code after your help

#include <iostream>
#include <fstream> 
using namespace std; 
  
int main() 
{ 
     const int N=7; 
     int k=3, i =0, j=0;
     int sum, MaxSum;
     int S1[N] = {10, -3, -1, 6, 4, 1, -10}; 


     for (i=0; i<N-k+1; i++) {
          sum = 0;
          for (j=0+i; j<i+k; j++) {
               sum += S1[j];
		
               if ( i == 0 || sum > MaxSum ) MaxSum = sum;
	
          }
     }
     cout << MaxSum << endl;
	   
return 0; 
} 

[lastchance]

I think you need to swap your current lines 18 and 20. What you do with the complete sum needs to be outside the j loop.

Otherwise, well done.