Anything wrong with this function?

[Anonymous]

Came across some code summarized as follows:

char const* MyClass::errToText(int err) const
{
switch (err)
{
case 0: return "No error";
case 1: return "Not enough";
case 2: return "Too much";
default: return "Unknown error";
}
}

Someone else said it was fine - was she right?

 

[Andrey Tarasevich]

Came across some code summarized as follows:

char const* MyClass::errToText(int err) const
{
switch (err)
{
case 0: return "No error";
case 1: return "Not enough";
case 2: return "Too much";
default: return "Unknown error";
}
}

Someone else said it was fine - was she right?

It depends on what kind of 'fine' you are asking about...

Formally the code is indeed perfectly fine.

One possible nitpick is that currently 'errToText' is a non-static
member function of class 'MyClass'. Since it doesn't need to access any
members of the class (at least in this form), maybe it should be turned
into a _static_ member function. Maybe not. Hard to say with such little
context.

 

[Grizlyk]

char const* MyClass::errToText(int err) const

Say "const" if you are not going to change variable
char const* MyClass::errToText(const int err) const

 

[Rolf Magnus]

It depends on what kind of 'fine' you are asking about...

Formally the code is indeed perfectly fine.

One possible nitpick is that currently 'errToText' is a non-static
member function of class 'MyClass'. Since it doesn't need to access any
members of the class (at least in this form), maybe it should be turned
into a _static_ member function. Maybe not. Hard to say with such little
context.

Another style issue here is that magic numbers are used. I'd make something
like:

enum Error
{
NoError, NotEnough, TooMuch
};

and then use that instead of int and magic numbers for the errors.

 

[Sylvester Hesp]

Say "const" if you are not going to change variable
char const* MyClass::errToText(const int err) const

The parameter is local for the function, nobody's going to care whether that
int is const or not. In fact, the standard doesn't care either. These two
function declarations declare the same function:

void foo(int);
void foo(const int);

- Sylvester

 

[frame]

Came across some code summarized as follows:

char const* MyClass::errToText(int err) const
{
switch (err)
{
case 0: return "No error";
case 1: return "Not enough";
case 2: return "Too much";
default: return "Unknown error";
}

}

Someone else said it was fine - was she right?

I have a doubt:

Aren't the above character strings "No error", "Not enough", ... local
to the member function MyClass::errToText(...)? If they are local,
then won't the returned pointers be dangling upon an exit from this
function?

 

[Gavin Deane]

I have a doubt:

Aren't the above character strings "No error", "Not enough", ... local
to the member function MyClass::errToText(...)? If they are local,
then won't the returned pointers be dangling upon an exit from this
function?

No. String literals are allocated statically. The returned pointer
will be valid throughout the lifetime of the program.

Gavin Deane

 

[Anonymous]

Another style issue here is that magic numbers are used. I'd make something
like:

enum Error
{
NoError, NotEnough, TooMuch
};

and then use that instead of int and magic numbers for the errors.

I was unused to the return of string literals, but eventually
found a reference to this style in C. I had always written
similar code (in C) in table style - something like this:

#define numberof(arr) ((unsigned) (sizeof(arr) / sizeof(arr[0])))

enum MCErrorCode
{
MC_ERROR_NONE = 0,
MC_ERROR_2FEW = 1,
MC_ERROR_2MUCH = 2,
MC_ERROR_UNKNOWN = -1
};

const char* MyClass::errToText(int err) const
{
static struct MCError
{
MCErrorCode errcode;
char* errmsg;
} mcErrors [] =
{
{ MC_ERROR_NONE, "No error" },
{ MC_ERROR_2FEW, "Not enough" },
{ MC_ERROR_2MUCH, "Too much" },
{ MC_ERROR_UNKNOWN, "Unknown error" }
};

const int nerrs = numberof(mcErrors);

int i;
for (i = 0; i < nerrs-1; i++)
{
if (err == mcErrors.errcode)
break;
}

return mcErrors.errmsg;
}

 

[Sylvester Hesp]

#define numberof(arr) ((unsigned) (sizeof(arr) / sizeof(arr[0])))

template<class T, size_t N> size_t numberof(T (&)[N])
{
return N;
}

Difference: it only works for actual arrays (compile error otherwise) and it
complies to regular name lookup rules

- Sylvester

 

[Pete Becker]

#define numberof(arr) ((unsigned) (sizeof(arr) / sizeof(arr[0])))

template<class T, size_t N> size_t numberof(T (&)[N])
{
return N;
}

Difference: it only works for actual arrays (compile error otherwise) and it
complies to regular name lookup rules

And it's not a compile-time constant.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

 

[Rolf Magnus]

I have a doubt:

Aren't the above character strings "No error", "Not enough", ... local
to the member function MyClass::errToText(...)?

No. They have static storage duration.

 

[Grizlyk]

The parameter is local for the function, nobody's going to care whether
that int is const or not. In fact, the standard doesn't care either.

It is bad only for standard, because it is really different function for
user. Explicit const type declaration is allow to you to detect errors in
your code, and can help to compiler to reduce unnecessary data copying for
some functions.

 

[Kai-Uwe Bux]

It is bad only for standard, because it is really different function for
user.

Not this one, this is only different for the implementor of the function:
the user passes by value and keeps the original anyway. In fact, the client
code has no way to observe from the outside, whether the parameter is
modified by the function internally.

Explicit const type declaration is allow to you to detect errors in
your code, and can help to compiler to reduce unnecessary data copying for
some functions.

Not this one. This one would just tie down the hands of implementors of the
function. Suppose you declared

int f ( const int i );

and at some point you realize that you can speed up the computation of the
result by modifying i internally. Tough luck. That is why the standard is
well-advised to say:

Note that the situation is completely different for parameters passed by
reference.


Best

Kai-Uwe Bux

 

[Default User]

No. String literals are allocated statically. The returned pointer
will be valid throughout the lifetime of the program.

Right. Now, the somewhat similar looking case:

char* f()
{
char arr[] = "Some text";

return arr;
}


Would indeed be returning a pointer to invalid memory.





Brian

 

[Andrey Tarasevich]

...

Not this one. This one would just tie down the hands of implementors of the
function. Suppose you declared

int f ( const int i );

and at some point you realize that you can speed up the computation of the
result by modifying i internally. Tough luck. That is why the standard is
well-advised to say:

...

Well, every decision is a trade off. I can see the benefit of distinguishing
between these two functions. Consider the following example

void foo(const int, const int);
void bar(const int, const int);

...
foo(a, b);
bar(a, b);

In this case the compiler has to generate code that passes the same set of
arguments to all three functions. On a typical compiler with stack-based
argument passing it would look as follows (just a sketch)

push a
push b
call foo
pop b, a

push a
push b
call bar
pop b, a

Functions 'foo' and 'bar' will use the values in the stack as their local
variables. If the parameters are declared as 'const', it means that these
functions are not allowed to modify them. If the compiler could know that from
outside of those functions, it could produce a much more compact and efficient code

push a
push b
call foo
call bar
pop b, a

Unfortunately with the current language specification this is not possible in
general case.

 

[Grizlyk]

Not this one. This one would just tie down the hands of implementors of
the
function. Suppose you declared

int f ( const int i );

and at some point you realize that you can speed up the computation of the
result by modifying i internally. Tough luck. That is why the standard is
well-advised to say:

Note that the situation is completely different for parameters passed by
reference.

Yes, const reference explicitly requires from compiler to share data, but
violating const modifier in "void foo(const int)" for most cases is just
error sign, because function declatation is interface and it is not good to
change interface of anything "on fly".

A man, who developed function interface said with the help of cost modifier,
that value must not change for implementation, because it is external
constant (as M_PI for example) and M_PI can not be equal to 6 or 7 for any
purpose, it is nonsence.

In our example we must not expect from "err" in "...::errToText(int err)"
changes, because err is original error code, we must not change it during
work. If we need. we can do it explicit like this:

int &err_memory_as_ordinary_data_storage=const_cast<int&>(err);

and work with "err_memory_as_ordinary_data_storage" in free manner.

I agree, that parameter "passed by value" can create separated copy of its
data, but I think for parameter "passed by const value" must not be duty to
create new copy of data, and in theory const data can be shared by compiler
as compiler can eliminate unnecessary call of copy ctor on return.

At least in my program I give to compiler chance to optimizing the const
value if it can. And i am even refusing to use ctor as ordinary function
call, because i think variables must be treated by compiler as variables and
compiler must _not_ expect from ctor any hidden activity, in addition to
creating variable.

And I think compiler must have keyword for explicit protect variables from
any optimizations, something like "volatile".

 

[Kai-Uwe Bux]

Yes, const reference explicitly requires from compiler to share data, but
violating const modifier in "void foo(const int)" for most cases is just
error sign, because function declatation is interface and it is not good
to change interface of anything "on fly".

A man, who developed function interface said with the help of cost
modifier, that value must not change for implementation, because it is
external constant (as M_PI for example) and M_PI can not be equal to 6 or
7 for any purpose, it is nonsence.

In our example we must not expect from "err" in "...::errToText(int err)"
changes, because err is original error code, we must not change it during
work.

No, it is not the original error code object that you must not change. It is
a local copy that you can change at will. Those are the rules of the
language.

If we need. we can do it explicit like this:

int &err_memory_as_ordinary_data_storage=const_cast<int&>(err);

and work with "err_memory_as_ordinary_data_storage" in free manner.

Huh? I don't follow. const_casting away constness for actual const objects
is undefined behavior. So if err is actually const, this code is dubious.

I agree, that parameter "passed by value" can create separated copy of its
data,

"can"? Unless the compiler proves that it can optimize away the copy, the
copy is guaranteed to be made.

but I think for parameter "passed by const value" must not be duty
to create new copy of data, and in theory const data can be shared by
compiler as compiler can eliminate unnecessary call of copy ctor on
return.

At least in my program I give to compiler chance to optimizing the const
value if it can.

A sufficiently smart compiler can do that already; although, I concede that
const can provide useful hints to the compiler in this regard.

And i am even refusing to use ctor as ordinary function
call, because i think variables must be treated by compiler as variables
and compiler must _not_ expect from ctor any hidden activity, in addition
to creating variable.

And I think compiler must have keyword for explicit protect variables from
any optimizations, something like "volatile".

Both issues seems to be unrelated to the const issue.


Best

Kai-Uwe Bux

 

[Grizlyk]

Kai-Uwe Bux wrotte:

No, it is not the original error code object that you must not change. It
is
a local copy that you can change at will. Those are the rules of the
language.

I think, declaring copy of any data (variable passed by value) as const, a
man, who have developed function interface, said that even this concrete
copy of the data _logically_ can not be changed, in spite of all other
copyes of the same object (look - any copy of M_PI can not be equal to 6).

Word _logically_ means that compiler must protect your internal function
implementation from logically wrong writing into the variable, even if the
writing do not disturb all other copyes of the variable, because the
variable has been passed by value.

Huh? I don't follow. const_casting away constness for actual const objects
is undefined behavior. So if err is actually const, this code is dubious.

I think, in some cases (for inline function for exmple) compiler can
undersand are you trying to write to const_casted object here or not. If
not, compiler can skip useless copy ctor, if yes, compiler can create copy,
making variable accessable to write.

"can"? Unless the compiler proves that it can optimize away the copy, the
copy is guaranteed to be made.

Yes, it is a question - to do always copy of variable passed by value or not
to do. It is possible, that there are known to C++ experts reasons of
concrete behaviour or there are random selected conventions assumed by
standard.

But i think the next: in theory, if we have a following code
--
struct X
{
int sign;
X():sign(0){}
X(cons X&):sign(1){}
};

void foo(const X);
X x;
void test(){ foo(x); }
--
in general case programmer must not expect, that inside "foo(x)" he will get
"copy of ::x" (x.sign==1), not the same as "::x" (x.sign==0), but he must
expect, that original "::x" will not see, that "foo(x)" has been called and
exited.

I can prove my opinion by reasons of parameter or variable declaration,
because "foo(const X)" request accesst to X, but do not request
"X::ctor(X&)" calling, so we must not expect and assuming hidden activity
from ctors.

Declaration "foo(const X)" requests own copy of X, but does require nothing
about relationships between the copy of X and other copyes of X.

In other side, declaration "foo(const X&)" garantees, that copy ctor never
will be called, so "foo(const X)" is not strictly opposite to "foo(const
X)". We could explicit write copy ctor like this:

void test(){ foo( X(x) ); }

but by accordance with reasons of variable usage and creating compiler can
have ability to eliminate "X(x)" and pass to "foo" global "::x" reference in
fact, to reduce size and speed.

It looks like we have no reasons to make extra copy of data, but here can be
special cases (when we must do it), so I think in order to force compiler to
make copy of data always in any concrete place, we must have any C++ keyword
for it, something like "volatile" keyword.

In spite of theory, there are many well-known limitations, when behaviour is
well defined, for example for functions with "extern C" linkage "foo(const
X)" will always make a stack copy of global "::x", but the stack copy can be
shared by series of "foo(const X)" calls, nothing to deny it, maybe
excluding standard.

 

[Kai-Uwe Bux]

Well, every decision is a trade off. I can see the benefit of
distinguishing between these two functions. Consider the following example

void foo(const int, const int);
void bar(const int, const int);

...
foo(a, b);
bar(a, b);

In this case the compiler has to generate code that passes the same set of
arguments to all three functions. On a typical compiler with stack-based
argument passing it would look as follows (just a sketch)

push a
push b
call foo
pop b, a

push a
push b
call bar
pop b, a

Functions 'foo' and 'bar' will use the values in the stack as their local
variables. If the parameters are declared as 'const', it means that these
functions are not allowed to modify them. If the compiler could know that
from outside of those functions, it could produce a much more compact and
efficient code

push a
push b
call foo
call bar
pop b, a

Unfortunately with the current language specification this is not possible
in general case.

Hm, you could use

void foo ( const int &, const int & );
void bar ( const int &, const int & );

and the compiler could detect that ints are small and not inheritable and
then decide to put copies on the stack that surely will remain unchanged. I
am not sure whether current C++ compilers would do this kind of
optimization or whether they will blindly push addresses onto the stack.


Best

Kai-Uwe Bux

 

[Gavin Deane]

Yes, const reference explicitly requires from compiler to share data, but
violating const modifier in "void foo(const int)" for most cases is just
error sign, because function declatation is interface and it is not good to
change interface of anything "on fly".

That argument is flawed. The things that you should not change "on the
fly" are the things that can affect existing client code. The const-
ness of an argument taken by value is not one of those things so in
this context is not part of the function's interface at all.

In our example we must not expect from "err" in "...::errToText(int err)"
changes, because err is original error code, we must not change it during
work. If we need. we can do it explicit like this:

int &err_memory_as_ordinary_data_storage=const_cast<int&>(err);

and work with "err_memory_as_ordinary_data_storage" in free manner.

If err is declared const, that will not work.
If err is not declared const, that will work, but is equivalent to and
far more cumbersome than
....::errToText(int& err)
But that doesn't help if what you want is to be able to modify the
value without affecting the original object. That is most easily
achieved by
....::errToText(int err)

Gavin Deane