what is wrong with my code?

D

davy.zou

I found the code for a program that is suppose to be able to work out
the sumation of i from 1 to a certain integer n.

however, when I implemented to code into Visual C++, it doesn't work.
The original sample code provided by the website:

unsigned int Sum (unsigned int n) {
unsigned int result = 0;
for (unsigned int i=1; i<=n; i++) {
result+=1;
return result;
}
}

and the weblink for this code is http://www.brpreiss.com/books/opus4/html/page37.html

now, I tried to include the whole main function, cin and cout all that
stuff so I can run it and test it. Now the code looks like this:
---------------------------------------------------------------------------------
1#include <iostream.h> //assignment number 5087
2
3unsigned int Sum (unsigned int n);
4
5unsigned int Sum (unsigned int n) {
6
7 cout<<"Please enter n: "<<endl;
8 cin>>n;
9
10 unsigned int result = 0;
11 for (unsigned int i=1; i<=n; i++) {
12 result+=1;
13 return result;
14 }
15}
16
17 void main {

a=Sum;
cout<<"Sum is "<<a<<endl;

}
-----------------------------------------------------------
Now the compiler tells me I have got two errors. The error messages
are:
C:\Documents and Settings\Owner\My Documents\University\Data
Structures and Algorithms\sample 1\Cpp1.cpp(17) : error C2182:
'main' : illegal use of type 'void'

C:\Documents and Settings\Owner\My Documents\University\Data
Structures and Algorithms\sample 1\Cpp1.cpp(17) : error C2239:
unexpected token '{' following declaration of 'main'.

In other words, there are two things wrong with line 17. one is and
illegal use of type void, which is weird since just about all of my
assignments so far have been done with void main. The other is some
"unexpected token". Somebody help me! I am about the drown in the vast
sea of confusion! :}
 
I

Ian Collins

16
17 void main {

a=Sum;
cout<<"Sum is "<<a<<endl;

}
-----------------------------------------------------------
Now the compiler tells me I have got two errors. The error messages
are:
C:\Documents and Settings\Owner\My Documents\University\Data
Structures and Algorithms\sample 1\Cpp1.cpp(17) : error C2182:
'main' : illegal use of type 'void'

C:\Documents and Settings\Owner\My Documents\University\Data
Structures and Algorithms\sample 1\Cpp1.cpp(17) : error C2239:
unexpected token '{' following declaration of 'main'.

In other words, there are two things wrong with line 17. one is and
illegal use of type void, which is weird since just about all of my
assignments so far have been done with void main. The other is some
"unexpected token". Somebody help me! I am about the drown in the vast
sea of confusion! :}
Ignoring the rest of the code; main returns int, so void main() is
ill-formed. You are also missing the parenthesis after "main".
 
D

davy.zou

Ignoring the rest of the code; main returns int, so void main() is
ill-formed. You are also missing the parenthesis after "main".


LOL. I can't believe it is something that simple. Thank you.
 
D

davy.zou

What does this mean?

C:\Documents and Settings\Owner\My Documents\University\Data
Structures and Algorithms\sample 1\Cpp1.cpp(14) : warning C4715:
'Sum' : not all control paths return a value
 
I

Ian Collins

What does this mean?

C:\Documents and Settings\Owner\My Documents\University\Data
Structures and Algorithms\sample 1\Cpp1.cpp(14) : warning C4715:
'Sum' : not all control paths return a value
Please retain some context.

If you look again, you will see that you have your return inside the for
loop. You probably want it outside of the loop.
 
D

davy.zou

Please retain some context.

If you look again, you will see that you have your return inside the for
loop. You probably want it outside of the loop.

how is it that your can reply like right after someone makes a post?
not that I am nor grateful or something, I am very grateful, but do
you like get paid to do this?
 
I

Ian Collins

Please don't quote people's signatures, the lines after the "-- ".
how is it that your can reply like right after someone makes a post?
not that I am nor grateful or something, I am very grateful, but do
you like get paid to do this?
I just happened to be sitting here!
 
O

Old Wolf

unsigned int Sum (unsigned int n) {
unsigned int result = 0;
for (unsigned int i=1; i<=n; i++) {
result+=1;
return result;
}
}

and the weblink for this code ishttp://www.brpreiss.com/books/opus4/html/page37.html

You've made several transcription errors:
* You added extra { }
* You changed "result += i" to "result+=1"
* You changed ++i to i++

The first two drastically change the effect of the code.
1#include <iostream.h> //assignment number 5087

(Please don't post line-number listings , it just
makes it harder for us to put your code in our
compilers).

There is no such standard header as <iostream.h> .
The C++ standard came out in 1998 so that suggests
your learning materials are 10 years old or more.
You could remedy this by writing:
#include <iostream>
using namespace std;

which has a similar effect to what some old compilers
did when you wrote #include said:
3unsigned int Sum (unsigned int n);
4
5unsigned int Sum (unsigned int n) {

Line 3 is redundant (although this isn't a terrible error).
17 void main {

Should be:
int main() {

main must return an int in Standard C++. While there
are some compilers that also allow it to return void,
you may find that your code fails when you try it on
a different compiler.

Should be:
a = Sum();

You need to use the brackets to indicate that the
function should be called.
 
T

terminator

Now the compiler tells me I have got two errors.


when you make a single mistake C++ nags a lot.That is most of time
when you fix an error several birds get killed with one stone.this
time is not an exception and you have one syntactical mistake:
17 void main {

looks like you have forggoten the pharanteses after function main:

line_17: void main(){//etc

(also you need remove line numbers.)
10 unsigned int result = 0;
11 for (unsigned int i=1; i<=n; i++) {
12 result+=1;
13 return result;
14 }

this algorithm wont work and the result will always be 1 .I suggest
that you move the return statement to the line after the end of the
for loop:

line_10: unsigned int result = 0;
line_11: for (unsigned int i=1; i<=n; i++) {
line_12: result+=1;
line_13: };
line_14: return result;


in fact since your loop has a single-line statement romoving the '{'
and '}' on line_11 and line_13 will not harm.
finally it is recommended -though not nessesary- that main returns
int.

cheers,
FM
 
D

davy.zou

This is my own C++ code for finding the sum of an arithmetics
sequence,

#include <iostream.h>
void main () {
int i, n, result=0;
while (true) {
cout<<"enter n"<<endl;
cin>>n;
if ((n==-999) || (n==0)) {
break;
}
for (i=1; i<=n; i++) {
result+=i;
}
cout<<"result is"<<result<<endl;
}
}

As you can see the variable "result" needs to be cleared at the end of
each while loop or the result from the previous loop will be carried
on. hence, the summation of 1 through 4 will be greater then the
summation of 1 through 5 if I worked with 1 through 5 first, 1 through
4 second.
 
M

Markus Schoder

This is my own C++ code for finding the sum of an arithmetics sequence,

#include <iostream.h>
void main () {
int i, n, result=0;
while (true) {
cout<<"enter n"<<endl;
cin>>n;
if ((n==-999) || (n==0)) {
break;
}

Put
result = 0;

here or replace the for loop with

result = n % 2 ? (n+1)/2 * n : n/2 * (n+1);
 
T

terminator

Put
result = 0;

here or replace the for loop with

result = n % 2 ? (n+1)/2 * n : n/2 * (n+1);

a better solution:

inline unsigned long sum(unsigned short n){
return (unsigned long)n*--n>>1;
};

now call sum wherever you need.

Cheers,
FM
 
J

James Kanze

a better solution:
inline unsigned long sum(unsigned short n){
return (unsigned long)n*--n>>1;

The above line has undefined behavior, and will give different
results with different compilers, or different levels of
optimization with the same compiler.

If you want a function, then use the expression Markus proposed:

int
sum( int n )
{
return n % 2 ? (n+1)/2 * n : n/2 * (n+1) ;
}

You can simplify it if you know that n will be very small. Such
assumptions are generally counter-productive in a library
function, however. (You should probably add an assert
concerning n, to avoid possible overflow. But the
test---necessary though it be---was missing in the original code
as well.)
 
M

Markus Schoder

a better solution:

inline unsigned long sum(unsigned short n){
return (unsigned long)n*--n>>1;
};

now call sum wherever you need.

This has undefined behaviour. If n*(n-1)>>1 was intended it is also
mathematically wrong.

Also why use "unsigned short"? That is 16 bit on many platforms and long
may be 64 bit. Using >> to divide by a constant power of 2 is also
unnecessary since compilers will easily optimize that themselves.

The reason I used the ?: construct is to avoid overflow for large n by
the way.
 
T

terminator

This has undefined behaviour.

Are u certain?
If n*(n-1)>>1 was intended it is also
mathematically wrong.

Yes this one is so silly a mistake.I should have written this:

return (n*(long)(n+1))>>1;
may be 64 bit. Using >> to divide by a constant power of 2 is also
unnecessary since compilers will easily optimize that themselves.

I would like to force compiler to do it the faster way.Maybe someone
uses an old compiler.
Also why use "unsigned short"? That is 16 bit on many platforms and long
The reason I used the ?: construct is to avoid overflow for large n by
the way.

the result of integer multiplication is as many bits long as the sum
of nessasary bits for operands;So in general we can assume that result
of integer multiplication is twice its operands in size.On many
machines (all of them ,as well as I know)'long' is more than twice
'short' in size;therefore no bounds check for an unsigned will be
nessesary.In your solution overflow is not fully prevented.consider:

unsigned n= ~(1<<(sizeof(int)-1));
unsigned result=(n+1)/2*n;/*overflow:result is 2*sizeof(int)-1 bits
long where we only have sizeof(int) bits. */
result=(n/2*(n+1);//overflow again
__int64 result 64=(n+1)/2*n;//ok:enough bits available now.

regards,
FM
 
M

Markus Schoder

Are u certain?

Yes. I think this is in the FAQ.
the result of integer multiplication is as many bits long as the sum of
nessasary bits for operands;So in general we can assume that result of
integer multiplication is twice its operands in size.On many machines
(all of them ,as well as I know)'long' is more than twice 'short' in
size;therefore no bounds check for an unsigned will be nessesary.In your
solution overflow is not fully prevented.consider:

unsigned n= ~(1<<(sizeof(int)-1));
unsigned result=(n+1)/2*n;/*overflow:result is 2*sizeof(int)-1 bits long
where we only have sizeof(int) bits. */ result=(n/2*(n+1);//overflow
again
__int64 result 64=(n+1)/2*n;//ok:enough bits available now.

sizeof gives the number of _bytes_ not bits but I get the idea.

My point was to have code that is equivalent to the OP's who used int.
The ?: will not prevent overflow but guarantees that it will not overflow
for smaller n than the OP's code.

Using short as input with a long result may prevent overflow from
happening but may also limit the input range (this is all platform
specific since there is no guarantee that sizeof(short) < sizeof(long))
and __int64 or long long or int64_t is non-standard.
 
J

James Kanze

Are u certain?

The standard says it is. In practice, different compilers give
very different results.
Yes this one is so silly a mistake.I should have written this:
return (n*(long)(n+1))>>1;

What does the cast change?

And of course, this may overflow although the results will fit.
I would like to force compiler to do it the faster way.Maybe someone
uses an old compiler.

How do you know it is the faster way? (I've actually worked on
a machine, the NSC 32000, where it wasn't.) This sort of coding
is really rather amateurish, and wouldn't be allowed in any
reasonable company.
the result of integer multiplication is as many bits long as the sum
of nessasary bits for operands;So in general we can assume that result
of integer multiplication is twice its operands in size.On many
machines (all of them ,as well as I know)'long' is more than twice
'short' in size;therefore no bounds check for an unsigned will be
nessesary.

You obviously don't know very many machines, then. There are
machines on which short is the same size as long.
In your solution overflow is not fully prevented.

If the results won't fit, there's not much you can do. His
solution won't overflow if the results are representable. Yours
will overflow for intermediate values even when the correct
results are representable.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,598
Members
45,151
Latest member
JaclynMarl
Top