patterncModerate
isPermutation method
Viewed 0 times
ispermutationmethodstackoverflow
Problem
I wrote an
isPermute method. Can I get some tips and advice on better coding style?#include
#include
#include
#include
bool isPermute(char*, char*);
char* sort(char*);
int main(int argv, char **argc)
{
printf("%s and %s are %spermutations of each other. \n", argc[1], argc[2], isPermute(argc[1], argc[2]) ?
"" : "not ");
return 0;
}
bool isPermute(char* s1, char* s2)
{
int size1 = strlen(s1);
int size2 = strlen(s2);
//default cases
if((size1 != size2) || (size1 == 0) || (size2==0))
return false;
if(strcmp(sort(s1), sort(s2)))
return false;
else
return true;
}
char* sort(char* str1)
{
int d=0, size = strlen(str1);
char character;
char *original = str1;
char *result = (char *)malloc(size);
for ( character = 'a' ; character <= 'z' ; character++ )
{
int i;
for ( i = 0 ; i < size ; i++ )
{
if ( str1[i] == character )
{
result[d] = str1[i];
d++;
}
}
str1 = original;
}
return result;
}Solution
First off:
You could just move the function implementation over the main method, avoiding to have these declarations; however, that isn't really a big deal.
The convention for
It normally is the opposite:
So it really should be:
Next:
You never checked if the program was actually passed two parameters, and so instead it should be (argc is the arg count from now on, and argv is the arguments)
The following:
can be trimmed down to be just:
The following:
You don't need to have the explicit cast
Which doesn't detract from clarity in any real way.
Also, I've noticed that you are declaring variables for the for loops outside the loop, even though that isn't necessary (unless you are using C89, in which case, ignore this)
So this:
would become
The same thing essentially applies for the other for loop.
would become
bool isPermute(char*, char*);
char* sort(char*);You could just move the function implementation over the main method, avoiding to have these declarations; however, that isn't really a big deal.
The convention for
int main isn't:int main(int argv, char **argc)It normally is the opposite:
argc is for the arg count and argv is for the arguments passed to the program. So it really should be:
int main(int argc, char **argv)Next:
printf("%s and %s are %spermutations of each other. \n", argc[1], argc[2],
isPermute(argc[1], argc[2]) ? "" : "not ");You never checked if the program was actually passed two parameters, and so instead it should be (argc is the arg count from now on, and argv is the arguments)
if(argc == 3)
{
printf("%s and %s are %spermutations of each other. \n", argv[1], argv[2],
isPermute(argv[1], argv[2]) ? "" : "not ");
}
else
{
printf("Incorrect amount of parameters passed!\n");
return -1;
}The following:
if(strcmp(sort(s1), sort(s2)))
return false;
else
return true;can be trimmed down to be just:
return !strcmp(sort(s1), sort(s2));The following:
char *result = (char *)malloc(size);You don't need to have the explicit cast
(char ) as malloc returns a void, so it can just bechar *result = malloc(size);Which doesn't detract from clarity in any real way.
Also, I've noticed that you are declaring variables for the for loops outside the loop, even though that isn't necessary (unless you are using C89, in which case, ignore this)
So this:
for ( character = 'a' ; character <= 'z' ; character++ )
{
...
}would become
for (char character = 'a'; character <= 'z'; character++)
{
...
}The same thing essentially applies for the other for loop.
for ( i = 0 ; i < size ; i++ )
{
...
}would become
for (int i = 0; i < size; i++)
{
...
}Code Snippets
bool isPermute(char*, char*);
char* sort(char*);int main(int argv, char **argc)int main(int argc, char **argv)printf("%s and %s are %spermutations of each other. \n", argc[1], argc[2],
isPermute(argc[1], argc[2]) ? "" : "not ");if(argc == 3)
{
printf("%s and %s are %spermutations of each other. \n", argv[1], argv[2],
isPermute(argv[1], argv[2]) ? "" : "not ");
}
else
{
printf("Incorrect amount of parameters passed!\n");
return -1;
}Context
StackExchange Code Review Q#115287, answer score: 11
Revisions (0)
No revisions yet.