Anything wrong with this function?

**Andrey Tarasevich** · Feb 2 '07, 09:55 AM

Re: Anything wrong with this function?

Anonymous wrote:

Came across some code summarized as follows:
>
char const* MyClass::errToT ext(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.

--
Best regards,
Andrey Tarasevich

**Grizlyk** · Feb 2 '07, 10:35 AM

Re: Anything wrong with this function?

Anonymous wrote:

>
char const* MyClass::errToT ext(int err) const

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

{
switch (err)
{
case 0: return "No error";
case 1: return "Not enough";
case 2: return "Too much";
default: return "Unknown error";
}
}

--
Maksim A Polyanin

**Rolf Magnus** · Feb 2 '07, 12:05 PM

Re: Anything wrong with this function?

Andrey Tarasevich wrote:

Anonymous wrote:

>Came across some code summarized as follows:
>>
>char const* MyClass::errToT ext(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.

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** · Feb 2 '07, 01:45 PM

Re: Anything wrong with this function?

"Grizlyk" <grizlyk1@yande x.ruwrote in message news:epv3jg$mg$ 1@aioe.org...

Anonymous wrote:

>>
>char const* MyClass::errToT ext(int err) const

>
Say "const" if you are not going to change variable
char const* MyClass::errToT ext(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** · Feb 2 '07, 03:35 PM

Re: Anything wrong with this function?

On Feb 2, 2:07 pm, Anonymous <inetl...@hotma il.comwrote:

Came across some code summarized as follows:
>
char const* MyClass::errToT ext(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::errToT ext(...)? If they are local,
then won't the returned pointers be dangling upon an exit from this
function?

**Gavin Deane** · Feb 2 '07, 03:45 PM

Re: Anything wrong with this function?

On 2 Feb, 15:27, "frame" <v.sri...@gmail .comwrote:

On Feb 2, 2:07 pm, Anonymous <inetl...@hotma il.comwrote:
>

Came across some code summarized as follows:

>

char const* MyClass::errToT ext(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::errToT ext(...)? 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** · Feb 2 '07, 04:15 PM

Re: Anything wrong with this function?

In article <epv8pv$2vi$01$ 1@news.t-online.com>,
Rolf Magnus <ramagnus@t-online.dewrote:

Andrey Tarasevich wrote:
>

Anonymous wrote:

Came across some code summarized as follows:
>
char const* MyClass::errToT ext(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.

>
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_UNKNOW N = -1
};

const char* MyClass::errToT ext(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_UNKNOW N, "Unknown error" }
};

const int nerrs = numberof(mcErro rs);

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

return mcErrors[i].errmsg;
}

**Sylvester Hesp** · Feb 2 '07, 04:25 PM

Re: Anything wrong with this function?

"Anonymous" <inetladd@hotma il.comwrote in message
news:inetladd-01DDB9.10024202 022007@news-server.houston. rr.com...

In article <epv8pv$2vi$01$ 1@news.t-online.com>,
Rolf Magnus <ramagnus@t-online.dewrote:
>
#define numberof(arr) ((unsigned) (sizeof(arr) / sizeof(arr[0])))
>

template<class T, size_t Nsize_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** · Feb 2 '07, 04:35 PM

Re: Anything wrong with this function?

Sylvester Hesp wrote:

"Anonymous" <inetladd@hotma il.comwrote in message
news:inetladd-01DDB9.10024202 022007@news-server.houston. rr.com...

>In article <epv8pv$2vi$01$ 1@news.t-online.com>,
>Rolf Magnus <ramagnus@t-online.dewrote:
>>
>#define numberof(arr) ((unsigned) (sizeof(arr) / sizeof(arr[0])))
>>

>
template<class T, size_t Nsize_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** · Feb 2 '07, 05:15 PM

Re: Anything wrong with this function?

frame wrote:

On Feb 2, 2:07 pm, Anonymous <inetl...@hotma il.comwrote:

>Came across some code summarized as follows:
>>
>char const* MyClass::errToT ext(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::errToT ext(...)?

No. They have static storage duration.

**Grizlyk** · Feb 2 '07, 06:25 PM

Re: Anything wrong with this function?

Sylvester Hesp wrote:

>Anonymous wrote:

>>>
>>char const* MyClass::errToT ext(int err) const

>>
>Say "const" if you are not going to change variable
>char const* MyClass::errToT ext(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.

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.

These two function declarations declare the same function:
>
void foo(int);
void foo(const int);
>

--
Maksim A Polyanin

**Kai-Uwe Bux** · Feb 2 '07, 06:55 PM

Re: Anything wrong with this function?

Grizlyk wrote:

Sylvester Hesp wrote:

>>Anonymous wrote:
>>>>
>>>char const* MyClass::errToT ext(int err) const
>>>
>>Say "const" if you are not going to change variable
>>char const* MyClass::errToT ext(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.

>
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:

>These two function declarations declare the same function:
>>
>void foo(int);
>void foo(const int);

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

Best

Kai-Uwe Bux

**Default User** · Feb 2 '07, 07:45 PM

Re: Anything wrong with this function?

Gavin Deane wrote:

On 2 Feb, 15:27, "frame" <v.sri...@gmail .comwrote:

On Feb 2, 2:07 pm, Anonymous <inetl...@hotma il.comwrote:

Came across some code summarized as follows:

char const* MyClass::errToT ext(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::errToT ext(...)? 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.

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** · Feb 2 '07, 08:25 PM

Re: Anything wrong with this function?

Kai-Uwe Bux wrote:

...

>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:
>

>>These two function declarations declare the same function:
>>>
>>void foo(int);
>>void foo(const int);

...

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.

--
Best regards,
Andrey Tarasevich

Anything wrong with this function?

Anything wrong with this function?

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment