Return to Answer

• 1

• 97.7k
• 5
• 126
• 341

###Did you ask any questions before you started?

It seems like there are a couple of holes in the design that need firming up.

1. Is there a maximum run length? (ie is single digit lengths enough)
2. Does the input string only contain alpha? (ie no numbers).
3. Do you encode single character runs? (ababab => a1b1a1b1a1b1 or ababab)

###General comments

The algorithm you are using is too complex.
Some of the answers on SO show you how to simplify the algorithm so I will not repeat that here (they are all good).

###Code comments

Don't put two variables on a line:

int charRepeatCount = 0; bool isFirstChar = true;

Any function that begins with an underscore is a not a good idea.

_snprintf(repeatStr, 10, "%i", charRepeatCount);

The underscore means that it is implementation defined. If there are no alternatives fine. But always prefer standard functions to non-standard implementations hacks (especially if they were done by MS). In this specific case the standard one is better designed and make usage easier.

Also why are you printing to a character array then copying to the destination array?

 _snprintf(repeatStr, 10, "%i", charRepeatCount);
 int repeatCount = strlen(repeatStr);
 for (int i = 0; i < repeatCount; i++)
 {
 *target = repeatStr[i];
 target++;
 }

You can print directly to the destination:

 sprintf(target, "%i", charRepeatCount); // notice not snprintf because I
 // I don't know the size of the
 // target. That is comming up.
 // You should definately use the
 // snprintf() version if you can.

You have sections of repeated code (the bit above).
If you are repeating code you should break it into a separate function.

Interface design:

void formatstring(char* target, const char* source)

I have no idea how much space I am allowed to use in target. Either allow the function to allocate its own space (and return the pointer) or get a maximum size passed into the function. If you pass in a size the return value is usually the amount of space you would have needed to succeed. This allows the user to do pre-flighting to work out the size required.

int size = LokiFormatString(source, NULL, 0);
char* tareget = (char*) malloc(size + 1);
int size = LokiFormatString(source, target, size);