Should function argument be changed in function body?

M

Morgan Cheng

Hi All,

I was taught that argument valuse is not supposed to be changed in
function body. Say, below code is not good.
void foo1(int x)
{
x ++;
printf("x+1 = %d\n", x);
}
It should be "refactor-ed" to be
void foo2(int x)
{
int y = x;
y ++;
printf("x + 1 = %d\n", y);
}

Recently, I found one guy posted code like foo1 in one C lang forum. I
pointed out that it is not good code standard. Surprisingly, seems all
people criticise me as dogmatism. It is claimed that changing arugment
value will not affect caller code and it saves stack size. I understand
their point, but I DO remember it is commmon that argument is not
suppsoed to be changed.

Now I am confused. Is there anybody can give some explaination why it is
a code rule that argument should not be changed?


Thanks & Regards
 
C

CBFalconer

Morgan said:
.... snip ...

Now I am confused. Is there anybody can give some explaination
why it is a code rule that argument should not be changed?

The only reason not to modify arguments is that the initial value
is required later. Any such rule is pure foolishness. You should
think of arguments as externally initialized local variables (which
is precisely what they are). For example:

int putblanks(size_t n, FILE *f)
{
while (n--)
if (EOF == putc(' ', f)) return EOF;
return ' ';
}
 
C

Cong Wang

Morgan said:
Hi All,

I was taught that argument valuse is not supposed to be changed in
function body. Say, below code is not good.
void foo1(int x)
{
x ++;
printf("x+1 = %d\n", x);
}
It should be "refactor-ed" to be
void foo2(int x)
{
int y = x;
y ++;
printf("x + 1 = %d\n", y);
}

Recently, I found one guy posted code like foo1 in one C lang forum. I
pointed out that it is not good code standard. Surprisingly, seems all
people criticise me as dogmatism. It is claimed that changing arugment
value will not affect caller code and it saves stack size. I understand
their point, but I DO remember it is commmon that argument is not
suppsoed to be changed.

Now I am confused. Is there anybody can give some explaination why it is
a code rule that argument should not be changed?


Thanks & Regards

Yeah,changing argument values is really not a good habit.I wanna try to
explain it using asm,though I just BEGIN to learn it.
Use gcc to change the code above into asm, and look for the
differences.You
will find function arguments are pushed into 'stack' and the return
value is in %eax register.If you change the argument in the 4(%esp),it
is bad of course.Er,just so so.
If I am not right,correct this at once! I just have a try! :-(
 
M

Michael Mair

Morgan said:
Hi All,

I was taught that argument valuse is not supposed to be changed in
function body. Say, below code is not good.
void foo1(int x)
{
x ++;
printf("x+1 = %d\n", x);
}
It should be "refactor-ed" to be
void foo2(int x)
{
int y = x;
y ++;
printf("x + 1 = %d\n", y);
}

Recently, I found one guy posted code like foo1 in one C lang forum. I
pointed out that it is not good code standard. Surprisingly, seems all
people criticise me as dogmatism. It is claimed that changing arugment
value will not affect caller code and it saves stack size. I understand
their point, but I DO remember it is commmon that argument is not
suppsoed to be changed.

Now I am confused. Is there anybody can give some explaination why it is
a code rule that argument should not be changed?

As Chuck Falconer pointed out, the only pragmatical reason _not_ to
change an argument value is that you need it later on.
One _could_ probably argue also that an argument should not be "abused"
for another purpose, e.g. using an argument green_val to store the
result of a completely unrelated computation (instead of introducing
a temporary variable with an apt name, say wobble_factor).
These are the only reasons I can come up with right now.

Making your "rule" into a mandatory rule for a coding standard is indeed
unnecessary dogmatism bordering on sheer folly.


Cheers
Michael
 
M

Michael Mair

Cong said:
Yeah,changing argument values is really not a good habit.I wanna try to
explain it using asm,though I just BEGIN to learn it.
Use gcc to change the code above into asm, and look for the
differences.You
will find function arguments are pushed into 'stack' and the return
value is in %eax register.If you change the argument in the 4(%esp),it
is bad of course.Er,just so so.
If I am not right,correct this at once! I just have a try! :-(

You are not right.
This is a pseudo-efficiency, pseudo-clarity argument.
C is not assembler, so this does not play a role. At all.
<OT>
Apart from that: Where do you think variables come from/are stored to?
Translating the C code into assembler may well lead to loading an often
used argument into a register, retrieving it only from this register,
nothing forces the compiler to "leave" the variable in the stack.
</OT>

Cheers
Michael
 
P

pete

CBFalconer said:
The only reason not to modify arguments is that the initial value
is required later.

I don't think that's a good reason.
If the specifcation were to suddenly change from the code example

void foo1(int x)
{
x ++;
printf("x+1 = %d\n", x);
}

to
int foo1(int x);
with the return value being the initial value of x,
I would do it this way:

int foo1(int x)
{
const int y = x;

x ++;
printf("x+1 = %d\n", x);
return y;
}

I think the code is easier to read
if the computations use the parameters.

char *str_cpy(char *s1, const char *s2)
{
char *const p1 = s1;

do {
*s1++ = *s2;
} while (*s2++ != '\0');
return p1;
}

If the intial value of a parameter is important,
it's easy to just pop in const qualified object.
 
C

CBFalconer

pete said:
I don't think that's a good reason.
If the specifcation were to suddenly change from the code example

void foo1(int x)
{
x ++;
printf("x+1 = %d\n", x);
}

to
int foo1(int x);
with the return value being the initial value of x,
I would do it this way:

int foo1(int x)
{
const int y = x;

x ++;
printf("x+1 = %d\n", x);
return y;
}

I wouldn't. What has ++ got to do with anything? I would write:

int foo1(int x) {
printf("x+1 = %d\n", x+1);
return x;
}

easily read by non C programmers and which is also easily modified
to return void. I don't believe in returning the value of entry
parameters anyhow, because that leads to such ugly foulups as:

printf("%s %s\n", b, strcat(b, a));

You will never have this foulup using strlcat.
 
M

Morgan Cheng

CBFalconer wrote:


The only reason not to modify arguments is that the initial value
is required later. Any such rule is pure foolishness. You should

Well, I have some industry experience. All of the code standard requires
that arguments are not to be changed. Even in embedded system
development, this rule is still required. I believe this rule is good
for code maintenance.
 
G

gooch

Well, I have some industry experience. All of the code standard requires
that arguments are not to be changed. Even in embedded system
development, this rule is still required. I believe this rule is good
for code maintenance.

Well, I have some industry experience as well and have never come
across this in a coding standard.
 
E

E. Robert Tisdale

Morgan said:
I was taught that
argument values are not supposed to be changed in function body.
Say, below code is not good.

void foo1(int x) {
++x;
printf("x+1 = %d\n", x);
}

should be "refactor-ed" to be

void foo2(int x) {
int y = x;
++y;
printf("x + 1 = %d\n", y);
}

This is *not* refactoring. You changed the API.
Recently, I found one guy posted code like foo1 in one C lang forum.
I pointed out that it is not good code standard.
Surprisingly, it seems that everybody criticised me as dogmatic.
It is claimed that changing arugment value will not affect caller code
and it saves stack size. I understand their point
but I *do* remember that it is commmon that
argument is not suppsoed to be changed.

Now I am confused. Is there anybody who can give some explaination,
"Why it is a code rule that argument should not be changed?"

There are several possible sources of confusion.
First, a function should *not* modify any of its *actual* arguments.
Instead of

void f(int* p) {
++(*p);
}

you should write

int f(const int* p) {
return *p + 1;
}

The example that you have provided passes by value
so it cannot modify its actual argument.
The *formal* argument is a copy of the *actual* argument.
If the *formal* argument should not be modified,
you should have written

void foo2(const int x) {
int y = x;
++y;
printf("x + 1 = %d\n", y);
}

Strictly speaking, it is up to the programmer
to decide whether [formal] arguments passed by value
are local constants or local variables.
Generally, you will *not* want to make a copy

void foo3(const HugeObjectType* p) {
// Use *p but don't modify it.
}

Your shop may enforce a rule
that generally forbids modifying formal arguments
to avoid accidently modifying actual arguments
(arguments passed by reference [through a pointer])
but objects must sometimes be modified *in-place*

HugeObjectType* foo4(HugeObjectType* p) {
// Modify *p
return p;
}

For example

char *strcpy(char *dest, const char *src);
 
K

Kenneth Brody

Morgan said:
Well, I have some industry experience. All of the code standard requires
that arguments are not to be changed. Even in embedded system
development, this rule is still required. I believe this rule is good
for code maintenance.
[...]

What industry, and whose standards? What is the rationale?

--
+-------------------------+--------------------+-----------------------------+
| Kenneth J. Brody | www.hvcomputer.com | |
| kenbrody/at\spamcop.net | www.fptech.com | #include <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------------+
Don't e-mail me at: <mailto:[email protected]>
 
G

Gordon Burditt

Recently, I found one guy posted code like foo1 in one C lang forum.
Generally I prefer not to change the formal arguments to a function,
because sooner or later I want the original value (even if it's in
a debugger only) for some reason. I don't consider it a "rule".
There are several possible sources of confusion.
First, a function should *not* modify any of its *actual* arguments.

I'll disagree strongly here, if what you mean is that you shouldn't
modify what a pointer passed in as an argument points at. How,
otherwise, would you handle something like the second argument of
strtol() or strtod()? Or for that matter, any function whose purpose
is to copy strings (strcpy(), strcat(), sprintf(), etc.)?

Obviously, any such argument should be clearly documented. Occasionally
a pointed-at value is used as both input AND output (e.g. it's a
pointer to where you left off in the parsing, and the new value is
where it left off after parsing the next token, used occasionally
in code intended to be a reentrant version of strtok() with a
necessarily modified argument list).
Instead of

void f(int* p) {
++(*p);
}

you should write

int f(const int* p) {
return *p + 1;
}

That's nice if the return value isn't being used for something else.
It is common for a function to need to return both a value and a
status of whether it worked or not. Sometimes this can be combined
into one (e.g. return NULL if it failed, pointer to something
otherwise), but there are objections to overloading the return value
like that also). Sometimes it can't (like if you want to return
an indicator of WHY it failed, and introducing more global variables
used like errno is also frowned on).

Gordon L. Burditt
 
E

E. Robert Tisdale

Gordon said:
Generally I prefer not to change the formal arguments to a function,
because sooner or later I want the original value
(even if it's in a debugger only) for some reason.
I don't consider it a "rule".


I'll disagree strongly here, if what you mean is that
you shouldn't modify what a pointer passed in as an argument points at.
How, otherwise, would you handle something like
the second argument of strtol() or strtod()?
Or, for that matter, any function whose purpose is to copy strings
(strcpy(), strcat(), sprintf(), etc.)?

Obviously, any such argument should be clearly documented.
Occasionally, a pointed-at value is used as both input AND output
(e.g. it's a pointer to where you left off in the parsing
and the new value is where it left off after parsing the next token,
used occasionally in code intended to be a reentrant version of strtok()
with a necessarily modified argument list).



That's nice if the return value isn't being used for something else.
It is common for a function to need to return
both a value and a status of whether it worked or not.
Sometimes this can be combined into one
(e.g. return NULL if it failed, pointer to something otherwise)
but there are objections to overloading the return value like that also).
Sometimes it can't (like if you want to return an indicator of WHY it failed
and introducing more global variables used like errno is also frowned
on).

The preferred solution is to return both values in a struct as in

div_t div(int numerator, int denominator);
ldiv_t ldiv(long numerator, long denominator);
lldiv_t lldiv(long long numerator, long long denominator);

You should try to avoid passing a non const pointer to a function
unless the object must be modified *in-place*. Usually,
only container objects (arrays for example) are modified in-place --
strtol(), strtod(), strcpy(), strcat(), strtok(), etc.

C does not have a very good exception handling mechanism.
Using a function argument to return status is a hack.
Generally, you can't use the function in an expression
because you must check the status code first.
> cat main.c
#include <stdio.h>

typedef struct Pair {
int value;
int status;
} Pair;

inline static
Pair f(const int i) {
Pair p;
p.value = i + 13;
p.status = 0;
return p;
}

int main(int argc, char* argv[]) {
const
Pair p = f(42);
fprintf(stdout, "value = %d\tstatus = %d\n",
p.value, p.status);
return 0;
}
> gcc -Wall -std=c99 -pedantic -O2 -o main main.c
> ./main
value = 55 status = 0
 
G

Gordon Burditt

There are several possible sources of confusion.
on).

The preferred solution is to return both values in a struct as in

If the values have some reasonable relationship to each other, I'll
agree with that. If it's a status glued onto something else, I
don't.
div_t div(int numerator, int denominator);
But *NOT*:
struct status_and_quotient_t { int status,
long quotient} divv(long numerator, int denominator)
ldiv_t ldiv(long numerator, long denominator);
lldiv_t lldiv(long long numerator, long long denominator);

You should try to avoid passing a non const pointer to a function
unless the object must be modified *in-place*. Usually,
only container objects (arrays for example) are modified in-place --
strtol(), strtod(), strcpy(), strcat(), strtok(), etc.

What is the container object modified by strtod()? A char * is
modified, which points into an array which is NOT modified.
C does not have a very good exception handling mechanism.
Using a function argument to return status is a hack.

No, I want to use the return value to return status and
the function argument to return the results.
Generally, you can't use the function in an expression
because you must check the status code first.

Which is why I want to return the status and use the function
argument to return the results.
cat main.c
#include <stdio.h>

typedef struct Pair {
int value;
int status;
} Pair;

inline static
Pair f(const int i) {
Pair p;
p.value = i + 13;
p.status = 0;
return p;
}

int main(int argc, char* argv[]) {
const
Pair p = f(42);
fprintf(stdout, "value = %d\tstatus = %d\n",
p.value, p.status);
return 0;
}
gcc -Wall -std=c99 -pedantic -O2 -o main main.c
./main
value = 55 status = 0


Gordon L. Burditt
 
O

Old Wolf

CBFalconer said:
I don't believe in returning the value of entry parameters anyhow,
because that leads to such ugly foulups as:

printf("%s %s\n", b, strcat(b, a));

Amusingly, this is the same as:

printf("%s %s\n", strcat(b, a), b);
You will never have this foulup using strlcat.

I don't see your point. If the intention was to display the
string before and after, then you will need 2 printf calls
(and strcat's one is more concise than strlcat's):

printf("%s ", b);
printf("%s\n", strcat(b, a));
 
M

Mark McIntyre

Hi All,

I was taught that argument valuse is not supposed to be changed in
function body.

Thats silly. If you need the original value elsewhere in the function,
then fine, but otherwise there's no reason not to do this.
void foo1(int x)

since x is a copy of the original object, altering it has no effect
outside foo1.
It should be "refactor-ed" to be
void foo2(int x)
{
int y = x;

this merely creates extra memory and uses up some cpu time.
Now I am confused. Is there anybody can give some explaination why it is
a code rule that argument should not be changed?

There's no such rule. It might be a company-specific coding standard,
I could understand a company insisting that arguments were not
altered, or all function names began with a letter designating the
module name, or contained exactly 32 characters, or used a form of
hungarian notation. But there's no necessity for it other than company
practice.
 
T

Tim Rentsch

CBFalconer said:
Morgan said:
... snip ...

Now I am confused. Is there anybody can give some explaination
why it is a code rule that argument should not be changed?

The only reason not to modify arguments is that the initial value
is required later. Any such rule is pure foolishness. [snip...]

With all due respect to the esteemed gentleman, I think this position
is on the wrong side of the fence. There are lots of reasons why it
can be useful to keep the original argument value available, eg,

showing the value with printf for some tracing
writing an assertion relating the function's output and input
viewing the value while debugging

All of these uses (not to mention some others) tend to show up later
in the development cycle as much as they do earlier when the code is
first written. It often isn't possible to anticipate on first
writing when the original value will subsequently be needed.

On the flip side, there is rarely a really compelling argument
that parameter variables should be modified:

1. Only one line of code is needed to declare and
initialize a variable in the function body proper;
2. Such lines are written quite easily/mechanically;
3. With a reasonable optimizing compiler, there is no
run-time cost or storage cost associated with doing
so.

To be pragmatic, a rule that says one should *never* modify a
parameter variable might be considered a bit dogmatic. But as a
guideline it almost certainly helps more than it hurts. To say that
another way, during code review the writer of the code should have the
burden of defending a function body that modifies a parameter. As a
reviewer, my questions would be something like these:

1. Is the function body small (under 10 lines, say)?
2. Is the code simple, and clearly and obviously correct?
3. Is the code highly unlikely to have to change in the
foreseeable future?

If the answers to any of the questions is NO, then it's almost
certainly better to follow the guideline and not modify any of
the parameter variables.
 
C

CBFalconer

Tim said:
.... snip ...

To be pragmatic, a rule that says one should *never* modify a
parameter variable might be considered a bit dogmatic. But as a
guideline it almost certainly helps more than it hurts. To say
that another way, during code review the writer of the code should
have the burden of defending a function body that modifies a
parameter. As a reviewer, my questions would be something like
these:

1. Is the function body small (under 10 lines, say)?
2. Is the code simple, and clearly and obviously correct?
3. Is the code highly unlikely to have to change in the
foreseeable future?

If the answers to any of the questions is NO, then it's almost
certainly better to follow the guideline and not modify any of
the parameter variables.

And I would say a better guide is to make the answers be YES. The
sort of usages you listed earlier in the snipped portion come under
the heading of "needed later" to me. While something like:

void foo(T param) {
T saveparam = param;

/* machinations */
}

only requires one extra line, it does require extra code.
Certainly that code may get optimized away, and very likely will if
saveparam is never used. Meanwhile it is just one more thing to
confuse a maintainer, and one more expansion of the source code.
It is also one more place for an optimizer to become confused.

All of this is basically just another style war. We have expressed
opinions, together with some justifications for such, and thus have
divided the world into non-cretins and cretins (where obviously
cretins disagree with me :).
 
L

Lawrence Kirby

On Tue, 19 Jul 2005 12:39:27 -0700, E. Robert Tisdale wrote:

....
on).

The preferred solution is to return both values in a struct as in

That is a possible alternative but in practice is rarely the "preferred
option".
div_t div(int numerator, int denominator);
ldiv_t ldiv(long numerator, long denominator);
lldiv_t lldiv(long long numerator, long long denominator);

Yes, this is the classic example of this form, but you'll note that these
are just 3 variations of the same thing. There are few other
examples. It works OK here because there are just 2 integer values
involved but even with something as simple as this it can make the caller
code messy. From a good programming point of view it creates extra
coupling between the function and the caller because the caller has to
handle an extra type that isn't natural to the problem. Put it this way -
it is kludgy to involve a structure in a simple integer division operation.
You should try to avoid passing a non const pointer to a function unless
the object must be modified *in-place*.

Not really, this is a normal and correct technique in C. There is a
similar discussion about using references in C++ for this purpose, which
should be avoided in that language. Using pointers is one way of doing so. :)
Usually, only container objects
(arrays for example) are modified in-place -- strtol(), strtod(),
strcpy(), strcat(), strtok(), etc.

True, along with structures. But changing for example the char ** argument
of strtod() would not be an improvement.
C does not have a very good exception handling mechanism. Using a
function argument to return status is a hack. Generally, you can't use
the function in an expression because you must check the status code
first.

It is probably more common for this reason for the status to be the return
value and to pass back data through a pointer argument. That is especially
natural when arrays and structures are involved but still works fine for
basic types and is consistent.

Lawrence
 

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,733
Messages
2,569,440
Members
44,830
Latest member
ZADIva7383

Latest Threads

Top