String class and the use of char*/const char*

[goran.pusic]

Hi all,

we seem to be having a style issue in the team.

We have a string class that has char* and const char* conversion operators
(yes, i know we should be using std::string, please bear with me).

Some people in the team prefer that, when calling code like this:

retval func(..., const char* param, ...);

with an instance of our string class, we use an explicit cast, e.g.

mystring s(whatever);
func(..., (const char*)s, ...);

(as opposed to not using a cast).

Can I please have your opinion on this (good/bad/like/not like)? Insight?
(My purpose, obviously, is to show your responses to the team).

TIA,

Goran.

 

[Ian Collins]

Hi all,

we seem to be having a style issue in the team.

We have a string class that has char* and const char* conversion operators
(yes, i know we should be using std::string, please bear with me).

Some people in the team prefer that, when calling code like this:

retval func(..., const char* param, ...);

with an instance of our string class, we use an explicit cast, e.g.

mystring s(whatever);
func(..., (const char*)s, ...);

(as opposed to not using a cast).

Can I please have your opinion on this (good/bad/like/not like)? Insight?

Conversion operators are seldom a good idea and using C style casts is
just about never a good idea!

Your code would be a lot cleaner and you would save typing if you
adopted std::string's c_str() conversion function.

It goes without saying that you would be better off all round just using
std::string.

 

[goran.pusic]

Conversion operators are seldom a good idea and using C style casts is

just about never a good idea!



Your code would be a lot cleaner and you would save typing if you

adopted std::string's c_str() conversion function.

Thanks!

Indeed, we do have (an equivalent of) c_str() in said string class.

That's a third possibility.

We do use it, e.g. for "%s" formatting ( I know, we shouldn't be
having that either :-( - I didn't do it ;-) ).

Goran.

 

[Daniel]

It goes without saying that you would be better off all round just using
std::string.

Unless they actually needed a string class, of course, as opposed to an array of bytes.

Daniel

 

[Öö Tiib]

Unless they actually needed a string class, of course, as opposed to an array of bytes.

OP did write that they "have a string class". Ian quoted it ... you snipped it to doubt it?

 

[Daniel]

OP did write that they "have a string class". Ian quoted it ... you snipped it to doubt it?

I did want to "doubt" something, but not that.

Have a great day,
Daniel

 

[Seungbeom Kim]

Others have already suggested to use

s.c_str()

If this is not possible by some reason, then one should use as a last
resort:

static_cast<const char*>(s)

However, the latter is too verbose and the noise may hide any useful
content in the line, so it should be used only when interfacing legacy code
which you cannot easily change.

If the static_cast is too verbose and you cannot modify the class to add
the member function c_str(), then another option is a global function:

inline const char* c_str(const mystring& s)
{
return static_cast<const char*>(s);
}

func(..., c_str(s), ...);

 

[Alf P. Steinbach]

we seem to be having a style issue in the team.

We have a string class that has char* and const char* conversion operators
(yes, i know we should be using std::string, please bear with me).

Some people in the team prefer that, when calling code like this:

retval func(..., const char* param, ...);

with an instance of our string class, we use an explicit cast, e.g.

mystring s(whatever);
func(..., (const char*)s, ...);

(as opposed to not using a cast).

Can I please have your opinion on this (good/bad/like/not like)? Insight?
(My purpose, obviously, is to show your responses to the team).

Some times an explicit conversion is needed to select the intended
overload of a function.

For example,

struct S
{
operator char const* () const { return "Ta da"; }
};

#include <iostream>     // std::cout, std::cerr, std::endl
using namespace std;

void foo( void const* ) { cout << "foo receiced void pointer" << endl; }

template< class Char >
void foo( Char const* p ) { cout << "foo says: " << p << endl; }

auto main() -> int
{
foo( S() );
foo( static_cast<char const*>( S() ) );
}

[output]
foo receiced void pointer
foo says: Ta da
[/output]


In particular this is a problem with direct output to standard
iostreams, selecting the void* argument operator<< since implicit
conversion is not considered for template matching.

With that in mind,

* Where an explicit conversion is needed (e.g. as above), express it in
a safe way using static_cast or direct call of the conversion operator.
Do NOT use a C style cast. When the code is maintained the C style cast
can silently change meaning to something rather undesired...

* Where an explicit conversion isn't needed, just use the implicit
conversion, because this string class is used all over the place and
everybody knows about its behavior, and because it minimizes the code
impact of cleaning up those called functions to take string class type
arguments. One doesn't go on pointing out that the Earth is round.
Rather, it sounds pretty silly and gets annoying pretty fast when
someone can't refrain from pointing out the well known roundness of
Earth all the time, whenever there is any kind of reference to Earth.

Of course, as already remarked on in this thread, your firm would be
much better off using std::string. This is not the case for every custom
string class -- they have their uses, and std::string is about the worst
that COULD be standardized. But a class with implicit conversion to
non-const char*, well char based and encouraging uncontrolled outside
modification of its contents, that class sounds as having negative
practical value (creating work), so best ditch it. ;-)


Cheers & hth.,

- Alf

 

[Marcel Müller]

Hi all,

we seem to be having a style issue in the team.

We have a string class that has char* and const char* conversion operators

operator const char* is mostly fine, but as soon as you implement
operator char* your code is close to death. There are hundreds of
drawbacks. Most importantly your code can easily have undefined behavior
where no compiler can give you any warnings or errors. (Believe me, I
have written some string classes so far.)

From my experience as soon as a C++ program uses char* (without const)
is is mostly buggy. For instance it is likely to have buffer overrun
vulnerabilities.

(yes, i know we should be using std::string, please bear with me).

I, know, std::string is not always an option.

Some people in the team prefer that, when calling code like this:

retval func(..., const char* param, ...);

with an instance of our string class, we use an explicit cast, e.g.

As I said, const char* is mostly safe. You only have to check for
dangling references, but no more than with that C API anyway.

mystring s(whatever);
func(..., (const char*)s, ...);

This cast is the worst case, since i would also cast mystring* or
whatever to (const char*). Things you usually don't want to do without a
compiler warning.

The common way is to provide a conversion function like c_str().

Can I please have your opinion on this (good/bad/like/not like)? Insight?
(My purpose, obviously, is to show your responses to the team).

- Remove operator char*.
- Remove char* from the C** source wherever possible.
- If you really need to adopt C APIs with char* without O(n) copies
provide a function like char* mystring::allocate(size_t length) that
ensures length+1 writeable characters until any other non-const mystring
function call for this instance while s[length] should not receive
anything but \0.


Marcel