1
votes

I am refactoring an old C library, and am currently changing the external API so that it uses std::string instead of const char*.

ColumnType Table::getColType(const char *name) const
{
    int id = getColumnId(name) ;
    return getColType(id) ;
}

and

int Table::getColumnId (const char * col_name) const
{
    unsigned int i = 0;

    while ((i < m_table.num_cols) && (strcmp(m_table.cols[i]->name, col_name) != 0) )
        i++;

    if (i < m_table.num_cols)
        return i;
    else
        return -1;
}

To:

ColumnType Table::getColType(const std::string& name_in) const
{
    const char* name = name_in.c_str();
    int id = getColumnId(name) ;
    return getColType(id) ;
}

and 

int Table::getColumnId (const std::string& col_name_in) const
{
    const char* col_name = col_name_in.c_str();
    unsigned int i = 0;

    while ((i < m_table.num_cols) && (strcmp(m_table.cols[i]->name, col_name) != 0) )
        i++;

    if (i < m_table.num_cols)
        return i;
    else
        return -1;
}

In the new code, I am passing a const char* to functions that are now expecting a reference to const std::string. I know std::string can be initialised from a const char*, and the code compiles correctly (no warnings etc).

But I just want to make sure that I am not doing anything that will come to bite me later on (I18n issues aside).

In short - is what am doing "safe"?

3
why pass a const char*, and not the string itself? The only thing I can think of is that there will be unnecessary copies with each call... - Nim
sadly, C++ objects do not nicely cross library boundries. Or more specifically, it might fail if the two sides are compiled with different libraries, or by different compilers, or with different settings, or a different time of day... - Mooing Duck
@MooingDuck, sure; from his snippet though, looks like the call to getColumnId which accepts a std::string is passed a const char* - a bit redundant really... - Nim
@Nim: My bad, didn't see that. HomunculusReticulli: Why do you have functions that take strings when you immedately cast them back to const char* and then ignore them? That forces unneeded copies. - Mooing Duck
@Nim: Yes, I realized there were extra string copies being made - but I did not want to carryout a wholesale refactoring on 500+ functions. This is a 'first pass' (just dealing with the API), once that's working (which it is), then I intend to clean the internals (like getting rid of the unnecessary string copying etc. - Homunculus Reticulli

3 Answers

1
votes

1) No need in getColType to get the c_str() from the std::string, just pass the std::string& directly into getColumnId.

2) You should use the overridden equality operator or use std::string::compare directly instead of strcmp. See

1
votes

It's safe, in so far as, executed correctly, it shouldn't cause any harm.

Nevertheless, I don't think I would recommend going forward with this unless you can point to a substantial benefit.

What may be safer is to add a function that takes in the const string&, and does a straight pass-thru to the const char* function. That way, you're letting clients stay in terms of std::string& code without modifying the internals.

For example:

ColumnType Table::getColType(const std::string& name_in) const
{
    return getColType(name_in.c_str());
}
0
votes

Beware of std::strings containing NULLs. The C++ class is fine with them; NULL is not special. But C strings of course treat it as end-of-string. The The following are not the same:

if (std_string_a == std_string_b) { /* C++ way */ }

// vs.

const char *cstring_a = std_string_a.c_str(),
           *cstring_b = std_string_b.c_str();
if (0 == strcmp(a, b)) { /* C way */ }

When you mix and match, you have to worry about weird bugs arising when the C++ way says false, yet the C way says true.