The best way to do all this is get rid of all the pointers and use a string all of the way through:
std::string getFileName(int pc1_no,
const std::string & suite)
{
std::string pCard1 = "cards/" + suite + std::to_string(pc1_no) + ".bmp";
return pCard1;
}
or if building to older C++ standards where std::to_string is not available:
std::string getFileName(int pc1_no,
const std::string & suite)
{
std::stringstream pCard1;
pCard1<< "cards/" << suite << pc1_no << ".bmp";
return pCard1.str();
}
Rational
char pCard1[80];
is a local variable. It will die at the end of the function, so the function returns a pointer to invalid memory. Many bad things can happen as a result and few good. Watch out for the few good. They are liars waiting for the most opportune time to strike.
The simplest solution maintaining OP's structure is use a std::string to perform string manipulation inside the function.
std::string getFileName(int* pc1_no, char* suite)
{
std::string pCard1 = "cards/";
pCard1 += std::string(suite);
pCard1 += std::to_string(*pc1_no);
pCard1 += std::string(".bmp");
return pCard1;
}
The above is Horrible Code, both excessively verbose and inefficient, but we've already covered the right way in the intro. This is just a touch-point on the logical progression to that right way.
Faster and less complex is to take advantage of std::stringstream's ability to format c-style strings and numbers without any outside help. This approach is probably the best up until std::to_string became available in the C++11 standard.
std::string getFileName(int* pc1_no, char* suite)
{
std::stringstream pCard1;
pCard1<< "cards/" << suite << *pc1_no << ".bmp";
return pCard1.str();
}
There is the possibility of a performance penalty returning the string by value, but the compilers of the past few decades are good at detecting and taking advantage of opportunities to omit unnecessary copying and employing move semantics behind the scenes.
Returning string by value is also far less error-prone than dynamically allocating storage and returning the storage to a caller with the expectation that the caller release it. Any performance penalty that may remain is highly likely to be worth the price. Profile the code to be sure.
Improving the function call:
Passing pc1_no in as a pointer is not helping in any way. Unless you need to modify the value inside the function, just pass by value. If you do need to change the value, prefer a reference.
std::string getFileName(int pc1_no, char* suite)
If you try to pass a string literal: eg:
getFileName(&somenumber, "string literal");
string literals may be in non-writable memory and are always const char * in C++. Passing a const value into a space that could attempt to change the value is bad form. This was allowed for backwards compatibility with C under older C++ Standards, though it may generate an warning, but is illegal in after the C++ 11 Standard.
If your function does not need to modify the contents of the char array, and this doesn't, it's a good practice to tag the string as const and allow the compiler to prevent accidents regardless of whether your compiler allows const to non-const assignments:
std::string getFileName(int pc1_no, const char* suite)
You may have more versatility if you use a reference to a const std::string as it is implicitly convertible from both const and non-const char arrays and allows the rest of your program to take advantage of the many benefits of std::string without needless calls to c_str and data.
std::string getFileName(int pc1_no,
const std::string & suite)
That brings us back to where this answer came in.
