Returning strings from functions in C++

Question

I have a function:

BOOL ReadWebPage(CONST TCHAR *sURL, TCHAR *sDataRead)
{
BOOL rv = FALSE
TCHAR *sFile = new TCHAR[1024]();
StringCchCopy(sFile, 1024, L"c:\\temp\\file.txt");
res = URLDownloadToFileW(NULL, sURL, sFile, 0, NULL);
if (res != S_OK)
    StringCchCopy(sDataRead, 16, L"Error"); 
else {
    StringCchCopy(sDataRead, 16, L"File exists on remote server");
    rv = TRUE; }
delete[] sFile;
return rv;
}

If it's *sDataRead, then the calling line would call this function like this:

ReadWebPage(L"http://www.example.com/test.txt", sTest);

If it's **sDataRead, then the calling line would call this function like this:

ReadWebPage(L"https://www.example.com/test.txt", &sTest);

My question is, should I make the declaration in the function for ReadWebPage as it is, *sDataRead, or is it more efficient to declare it as **sDataRead, or does it not make any difference?

If you want to return something from a function, returning it is typically the best way forward. — Bartek Banachewicz, Feb 08 '21 at 14:44
How about `BOOL ReadWebPage(CONST TCHAR *sURL, std::basic_string& sDataRead)`? — Some programmer dude, Feb 08 '21 at 14:44
@Someprogrammerdude That still doesn't return the string from this function. — Bartek Banachewicz, Feb 08 '21 at 14:45
And don't forget to actually copy to `sDataRead` even if the `URLDownloadToFileW` call is successful. And perhaps return `FALSE` on error? — Some programmer dude, Feb 08 '21 at 14:45
I don't want to return a string, I want the return string in the parameters of the function. — JeffR, Feb 08 '21 at 14:45
@JeffR Yes, which is precisely the wrong way to do it, especially in the case where the function doesn't fill a preallocated buffer but is responsible for the allocation itself. — Bartek Banachewicz, Feb 08 '21 at 14:45
@BartekBanachewicz, I am pre-allocating the sDataRead buffer from the calling function. — JeffR, Feb 08 '21 at 14:47
@JeffR But... you're still allocating another temporary buffer, which makes this whole exercise quite moot. You might as well return `sFile` pointer instead of deleting it. That alone will make it orders of magnitude less efficient than anything you hoped to gain by changing the signature. — Bartek Banachewicz, Feb 08 '21 at 14:49
@BartekBanachewicz, yes, I understand what you're saying, but I have many functions where the return value is either FALSE or a DWORD error, similar to many Windows APIs. I'm just trying to see what is best for returning a value in a parameter to the function. — JeffR, Feb 08 '21 at 14:51
Your question is incongruent to your description. Also why would you need a pointer to a pointer. — NonameLover, Feb 08 '21 at 14:55
@JeffR `StringCchCopy(sFile, 1024, L"c:\\temp\\file.txt");` -- This really isn't correct. If you know the string will be a wide string, why is sFile an array of TCHAR? A TCHAR is only going to be a wide string if the build type is Unicode, otherwise it is a narrow, ANSI string. You should just simply use wide-character types, and drop using the TCHAR if you know the application is Unicode. — PaulMcKenzie, Feb 08 '21 at 15:41
@PaulMcKenzie yes, just used for simplification in this code example. It's unicode, so TCHAR becomes WCHAR. — JeffR, Feb 08 '21 at 15:43
I’m voting to close this question because this belongs to https://codereview.stackexchange.com — Chnossos, Feb 08 '21 at 16:04

score 0 · Answer 1 · answered Feb 08 '21 at 14:55

Based on the comments, I think I can answer that quite decisively: this function is written in such a bad way that any gains you might hypothetically gain from changing the signature are completely irrelevant.

Besides, the proposed change (from TCHAR* to TCHAR**) is a semantic one. Sure, it involves one more indirection, but most importantly it allows you to modify the pointer value, which is not what you need or want at all if you're just filling a pre-allocated buffer.

Moreover, if the goal was to fill the pre-allocated buffer, that precludes the allocation in the function itself. Indeed, once you did the allocation, you could've easily returned sFile directly, avoiding a copy.

And if that could work (e.g. because you allocate the buffer right before this call, anyway), then the reasonable conclusion is to forego the out-parameters and simply return the result.

In fact, if you use standard containers, you can have your cake and eat it, too; by customizing the allocator used for the returned container you can make it use your desired memory without the need to use the out-parameter hack. That solution is a bit out of scope for this answer, though.

*customizing the allocator* is not a suggestion one should make lightly, in particular not when the problem is one of the standard category like this one. There are simpler off-the-shelf solutions than that. — j6t, Feb 08 '21 at 15:30
@j6t Hence why I'm just mentioning and not suggesting it as the first choice. — Bartek Banachewicz, Feb 08 '21 at 16:02

Returning strings from functions in C++

1 Answers1