Resolved: Strcat not appending a character

In this post, we will see how to resolve Strcat not appending a character

Question:

char* oledScreen::getCurrentTime(){
   char* hour = malloc(16);
   snprintf(hour, 16, "%d", getHour());

   char* minute = malloc(16);
   snprintf(minute, 16, "%d", getMinute());

   char* firstPart = strcat(getHour() < 10 ? strcat("0",hour) : hour, ":");
   const char* secondPart = getMinute() < 10 ? strcat("0",minute) : minute;

   return strcat(firstPart, secondPart);
};

I’m trying to append two integers, which I can obtain using getHour() and getMinute(). However, I need to check if one of these two are less than 10: if so, I need to append a 0 so that the output is such that: 0X, where X is getHour() or getMinute().
My problem is that it does not append the : character. For instance, if getHour() = 9 and getMinute() = 15. The output of getCurrentTime() is 0915 and not 09:15. Do you have any idea why this is like this?

Best Answer:

To begin with, strcat("0",hour) will lead to undefined behavior as you attempt to modify the literal string "0" which isn’t allowed.
Instead of using multiple strcat calls, why not simply create a string large enough to fit the result, and use snprintf to put contents into the string?
Using snprintf will also make it easier to optionally add a zero-prefix to the hour value.
Perhaps something like this:

// Allocate memory, and actually create the string
// 2 for hour, 2 for minutes, 1 for colon, 1 for terminator
char *result = malloc(6);

snprintf(result, 6, "%02d:%02d", getHour(), getMinute());

return result;

As mentioned in comments, a better solution would be to pass in the result array as an argument to the function. The would handle ownership of the result array better, and might not even need dynamic allocation.
_{[Note that the above is a pure C solution, it’s not even valid in C++. I wrote it before noticing that the code in the question is really C++.]}
Considering that you really seem to be programming in C++, not C, then a solution using string streams and std::string would be more appropriate:

std::string oledScreen::getCurrentTime(){
    std::ostringstream oss;

    oss << std::setw(2) << std::setfill('0') << getHour() << ':'
        << std::setw(2) << std::setfill('0') << getMinute();

    return oss.str();
}

If you have better answer, please add a comment about this, thank you!

Source: Stackoverflow.com

Question:

Best Answer:

Related