patterncMinor
Algorithm for Run Length Encoding - String Compression
Viewed 0 times
lengthcompressionalgorithmforencodingstringrun
Problem
I attempted a problem from the Cracking the Coding Interview book. The following input:
aabcccaaa should give the following output: a2b1c3a3. Any advice is much appreciated!#include
#include
#include
char* compress(char*);
int main(int argc, char** argv)
{
if(argc==2)
{
printf("The compression of %s is %s \n", argv[1],
compress(argv[1]));
}
else
printf("Not correct number of inputs.");
}
char* compress(char* str)
{
const size_t len = strlen(str);
char result[len];
//initialized to 0, so length check can be done later
memset(result, 0, len);
int counter;
int j = 0;
if(str==NULL || len == 0)
return 0;
result[0] = str[0];
for(size_t i = 0; i < len; i++)
{
counter = 1;//reset counter
//to make sure no array out of bounds exception occurs
if(result[j+1]==0)
result[++j] = counter + '0';
else
return str;
while(str[i] == str[i+1])
{
//to store number in char array, add the asci value
// of 0.
result[j] = (++counter + '0');
i++;
}
if(result[j+1]==0)
result[++j] = str[i+1];
else
return str;
}
result[++j] = '0';
//to avoid "function returns address of local variable" error
//to also avoid altering original argument
char* str1 = (char*)malloc(sizeof(result));
strcpy(str1, result);
return str1;
}Solution
-
You can eliminate that function prototype by defining
-
It's more common to first check the command line usage and then exit with a non-zero value if it's invalid. You should also print all errors to
-
You can probably check
-
The
-
The NULL character for
-
There's actually no need to cast the return type of
Moreover, you can avoid that aforementioned error by allocating
-
You never call
It may work to have something like this in
You can eliminate that function prototype by defining
main() last, but it's up to you.-
It's more common to first check the command line usage and then exit with a non-zero value if it's invalid. You should also print all errors to
stderr, not stdout (the latter is always used with printf()).if (argc != 2)
{
fprintf(stderr, "Not correct number of inputs.");
return -1;
}-
You can probably check
str right at the start, especially with the len assignment. If there is an error, then it may be clearer to return NULL instead of 0. At least that's what many library functions return specifically.-
The
while loop can instead be a for loop:for (; str[i] == str[i+1]; i++)
{
//to store number in char array, add the asci value
// of 0.
result[j] = (++counter + '0');
}-
The NULL character for
result is wrong; it should be '\0'.-
There's actually no need to cast the return type of
malloc() in C. Although malloc() does return a void, it's not required because any pointer type can be assigned to a void and is done "automatically" here.Moreover, you can avoid that aforementioned error by allocating
result after making sure that the original string is valid. This will also avoid the need to call strcpy() at the end and risk other bugs. Since result will not be local this way, you can return it from the function without losing the data.char* compress(char* str)
{
// check to make sure str is valid...
// assuming len is also valid
char* result = malloc(len);
// OR, use calloc() instead to get the memory zeroed
// thus no need for memset() or anything else
char* result = calloc(len, sizeof(char));
// probably also good to check for sufficient memory
// if insufficient, print error and abort
if (!result)
{
perror("malloc"); // or "calloc"
abort();
}
// do the compression...
// return it right away; no need to copy anything
return result;
}-
You never call
free() on str1, so this function leaks memory. You would have to do this in main() with the returned string, and it may be necessary to assign compress() to a const char to display before freeing afterward. This also means that compress() should return a const char, not a char*, preventing it from being modified further.It may work to have something like this in
main():int main(int argc, char** argv)
{
if (argc != 2)
{
// let the user know of the correct input
fprintf(stderr, "usage: %s string\n", argv[0]);
return -1;
}
const char* original = argv[1];
const char* compressed = compress(original);
printf("The compression of %s is %s \n", original, compressed);
free(compressed);
}Code Snippets
if (argc != 2)
{
fprintf(stderr, "Not correct number of inputs.");
return -1;
}for (; str[i] == str[i+1]; i++)
{
//to store number in char array, add the asci value
// of 0.
result[j] = (++counter + '0');
}char* compress(char* str)
{
// check to make sure str is valid...
// assuming len is also valid
char* result = malloc(len);
// OR, use calloc() instead to get the memory zeroed
// thus no need for memset() or anything else
char* result = calloc(len, sizeof(char));
// probably also good to check for sufficient memory
// if insufficient, print error and abort
if (!result)
{
perror("malloc"); // or "calloc"
abort();
}
// do the compression...
// return it right away; no need to copy anything
return result;
}int main(int argc, char** argv)
{
if (argc != 2)
{
// let the user know of the correct input
fprintf(stderr, "usage: %s string\n", argv[0]);
return -1;
}
const char* original = argv[1];
const char* compressed = compress(original);
printf("The compression of %s is %s \n", original, compressed);
free(compressed);
}Context
StackExchange Code Review Q#115538, answer score: 8
Revisions (0)
No revisions yet.