Back to top
Common mistakes with functionsComments and questions to John Rowe.
Functions are one of the key high-level tools in programming so
it's nt suprising that novice programmers find a lot of ways to go
wrong! These may be technical erros or just using functions in the wrong way.
Here we have gathered a number of examples of the sorts of erros we often see.
| Wrong
| Right
|
#include <stdio.h>
void badfun(int val) {
val = 100 + val ;
}
int main() {
int val = 1;
printf("Was: %d\n", val);
badfun(val);
printf("Now: %d\n", val);
return 0;
}
Step through this code
|
#include <stdio.h>
int goodfun(int val) {
val = 100 + val ;
return val;
}
int main() {
int val = 1;
printf("Was: %d\n", val);
val = goodfun(val);
printf("Now: %d\n", val);
return 0;
}
Step through this code
|
In the incorrect example on the left the coder appears to think
that the function call inside
main() is "passing the variable val
to badfun()" so it can change it. What is actually happening is
that the function call is calculating the value
of the mathematical expression val and passing that value to
badfun(). The fact that the two variables have
the same name has no significance.
The second, correct, example can be thought of as "pass the old
value of val to goodfun() for it to calculate the
new value and return it back to main()".
The sole purpose of parameters is to pass in a value which the function
needs to do its job and which it cannot calculate for itself. Do
not use them just to create local variables:
| Wrong
| Right
|
#include <stdio.h>
double mypow(double val, int expo, double result) {
result = 1.0;
if (val == 0.0 && expo < 0 ) {
printf("WARNING: ...\n");
return 0.0;
}
while (expo > 0) {
result = result * val;
expo = expo - 1;
}
while (expo < 0) {
result = result / val;
expo = expo + 1;
}
return result;
}
|
#include <stdio.h>
double mypow(double val, int expo) {
double result = 1.0;
if (val == 0.0 && expo < 0 ) {
printf("WARNING: 0and -ive\n");
return 0.0;
}
while (expo > 0) {
result = result * val;
expo = expo - 1;
}
while (expo < 0) {
result = result / val;
expo = expo + 1;
}
return result;
}
|
| Typical incorrect call:
| Typical correct call:
|
x = mypow(8.3, 4, 0.0);
|
x = mypow(8.3, 4);
| Explanation
In the first, incorrect, example we pass in the value 0.0 into
mypow() via the parameter (local variable)
result. However inside mypow()
we immediately
set
the value of result to 1.0 so there was no point
in passing in the value of 0.0.
HereIn the second example we correctly declare result to be
an ordinary local variable as it does not need to
have a value passed into it.
| |
Functions should always do the whole job, we should never say "The
person calling the function can do this" without a good reason.
We'll give two examples:
Functions not doing any required initialisation
In the previous example of the exponential function we had to set result
to 1.0. A common mistake would be to set result
to 1.0 in the function call rather than to do it inside the function:
| | Also wrong | Right
|
|---|
// Make sure result is set to 1.0
// before calling mypow()
double mypow(double val, int expo, double result) {
if (val == 0.0 && expo < 0 ) {
...
|
double mypow(double val, int expo) {
double result = 1.0;
if (val == 0.0 && expo < 0 ) {
...
I.e. the same as above
|
| Typical incorrect call:
|
|
x = mypow(8.3, 4, 1.0);
|
| Explanation
This is also incorrect as the third argument must always have the value
1.0 for the function to work correctly.
Functions must do the whole job, not just most of it,
and anything that
can be done inside the function must be done inside the function.
When there is a decision to be made
Sometimes it' a case of "if this condition is true call this function,
if not call another function" and the temptation is to make the person calling the
function(s) work out which is required. What we should do is to provide a small "wrapper"
function they can call that will do the work for them.
Example: a tabbed program
One example is programs that can have have tabs: it may be that
creating the first tab requires some setup that creating the second
and subsequent tabs don't and it may well the the right
decision to have two separate functions for this, makefirsttab()
and addtab(). What is not the right decision is to fail to
have a simple function that makes this choice for them
| | Wrong
| Right |
/*
* Insert warning here telling the user
* of these functions to be sure to check
* whether they need to call
* makefirsttab() or addtab()
*/
int makefirsttab(void) {
return tabid;
}
int addtab(void) {
return newtabid;
}
|
int newtab(void) {
if ( there_are_no_tabs )
return makefirsttab();
return addtab();
}
int makefirsttab(void) {
return tabid;
}
int addtab(void) {
return newtabid;
}
| Comment
The function newtab() is only three lines long and all it does
is choose whether to call makefirsttab() or addtab()!
We may think this is too simple to be a function but ask yourself
"which is easier to use: the code on the left or the code on the right?".
The purpose of a function is to make life easier for the person calling it.
|
This is a technical error: some users put the parameter type in the function call:
| Wrong
| Right
|
#include <stdio.h>
void myfun(int x);
int main() {
int x = 2, y = 3;
myfun(int x); // Wrong
myfun(int y); // Wrong
return 0;
}
|
#include <stdio.h>
void myfun(int x);
int main() {
int x = 2, y = 3;
myfun(x);
myfun(y);
myfun(x + 2*y);
return 0;
}
|
In the bad code the calls to myfun()
try to create brand-new variables
x and y, entirely unrelated to the variables
x and y in the
rest of main(),
just to pass their values to myfun().
This is meaningless and so is not allowed.
The third example in the good code illustrates the fact that
the arguments to function calls are just the values of
mathematical expressions.
We can illustrate this using our "exponent" code we have seen above.
| Wrong
| Right
|
#include <stdio.h>
int main() {
double val1=6.7, val2=-8.32, res1=1, res2=1;
int expo1 = -3, expo2=4;
if (val1 == 0.0 && expo1 < 0 ) {
printf("WARNING: 0 and -ive\n");
return 0.0;
}
while (expo1 > 0) {
res1 = res1 * val1;
expo1 = expo1 - 1;
}
while (expo1 < 0) {
res1 = res1 / val1;
expo1 = expo1 + 1;
}
if (val2 == 0.0 && expo2 < 0 ) {
printf("WARNING: 0 and -ive\n");
return 0.0;
}
while (expo2 > 0) {
res2 = res2 * val2;
expo2 = expo2 - 1;
}
while (expo2 < 0) {
res2 = res2 / val2;
expo2 = expo2 + 1;
}
return 0;
}
|
#include <stdio.h>
double mypow(double val, int expo) {
double result = 1.0;
if (val == 0.0 && expo < 0 ) {
printf("WARNING: 0and -ive\n");
return 0.0;
}
while (expo > 0) {
result = result * val;
expo = expo - 1;
}
while (expo < 0) {
result = result / val;
expo = expo + 1;
}
return result;
}
int main() {
double value1=6.7, value2=-8.32, res1, res2;
int expo1 = -3, expo2=4;
res1 = mypow(value1, expo1);
res2 = mypow(value2, expo2);
return 0;
| |
Notes
- We have been careful to declare and initialise
the variable result
inside the function rather than adding itas an
extra argument.
- As mentioned in the main notes, this code is complicated enough that
it's probably worth making into a function even if we only need to use it once.
- Later on we shall learn about arrays, which can also be useful
in these situations.
|
Here we reuse the initial "good" example:
|
|---|
| Wrong
| Right
|
#include
int main() {
int val = 1;
// Function body wrongly inside main()
int goodfun(int val) {
val = 100 + val ;
return val;
}
printf("Was: %d\n", val);
val = goodfun(val);
printf("Now: %d\n", val);
return 0;
}
|
#include <stdio.h>
int goodfun(int val) {
val = 100 + val ;
return val;
}
int main() {
int val = 1;
printf("Was: %d\n", val);
val = goodfun(val);
printf("Now: %d\n", val);
return 0;
}
Step through this code
| |
|
