Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

For the record, @Law29 @Law29 suggested another alternative in a comment:

As @DarthGizka @DarthGizka explained, instead of this:

For the record, @Law29 suggested another alternative in a comment:

As @DarthGizka explained, instead of this:

For the record, @Law29 suggested another alternative in a comment:

As @DarthGizka explained, instead of this:

added 548 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

For the record, @Law29 suggested another alternative in a comment:

You can also read from standard input. This lets you either type in words as they come to mind, or use a whole file (there are files of dictionary words, for example). Example:

#define MAX_WORD_SIZE 50
int main(int argc, char ** argv) { 
 char buf[MAX_WORD_SIZE]
 while (fgets (buf, MAX_WORD_SIZE, stdin)) {
 print_result(buf);
 }
}

For the record, @Law29 suggested another alternative in a comment:

You can also read from standard input. This lets you either type in words as they come to mind, or use a whole file (there are files of dictionary words, for example). Example:

#define MAX_WORD_SIZE 50
int main(int argc, char ** argv) { 
 char buf[MAX_WORD_SIZE]
 while (fgets (buf, MAX_WORD_SIZE, stdin)) {
 print_result(buf);
 }
}
added 848 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

Excessive looping

As @DarthGizka explained, instead of this:

for(char ch = 'a'; ch <= 'z'; ch++)
{
 for(int i=0; i < size; i++)
 {
 if(str[i] == ch)
 alpha[str[i]]++;
 }
}

This is identical, but without unnecessary looping:

for(int i=0; i < size; i++)
{
 alpha[str[i]]++;
}

Unnecessary conditions

The first condition is unnecessary:

 if(alpha[j] == 1 || (alpha[j]%2==1))

This is exactly the same:

 if(alpha[j]%2==1)

Too compact writing style

Instead of this:

 if(alpha[j]%2==1)

I suggest to put spaces around operators, and before ( in if statements:

 if (alpha[j] % 2 == 1)

Stop iterating when you already know the result

Once you find two characters with odd number of occurrences, you can stop iterating and return false. As such, you don't even need an int oddCount, but a bool seenOdd. So instead of this:

int oddCount = 0;
//count the number of times a letter only appears once
for (int j = 0; j < 256; j++)
{
 if (alpha[j] % 2 == 1) oddCount++;
}
//if there is more than one letter that only occurs, then it 
//cannot be a palindrome.
return oddCount <= 1;

You could write:

bool seenOdd = false;
// scan for odd number of occurrences, stop after seeing two
for (int j = 0; j < 256; j++)
{
 if (alpha[j] % 2 == 1) {
 if (seenOdd) return false;
 seenOdd = true;
 }
}
// less then 2 letters with odd number of occurrences, must be true
return true;

Excessive looping

As @DarthGizka explained, instead of this:

for(char ch = 'a'; ch <= 'z'; ch++)
{
 for(int i=0; i < size; i++)
 {
 if(str[i] == ch)
 alpha[str[i]]++;
 }
}

This is identical, but without unnecessary looping:

for(int i=0; i < size; i++)
{
 alpha[str[i]]++;
}

Unnecessary conditions

The first condition is unnecessary:

 if(alpha[j] == 1 || (alpha[j]%2==1))

This is exactly the same:

 if(alpha[j]%2==1)

Too compact writing style

Instead of this:

 if(alpha[j]%2==1)

I suggest to put spaces around operators, and before ( in if statements:

 if (alpha[j] % 2 == 1)

Stop iterating when you already know the result

Once you find two characters with odd number of occurrences, you can stop iterating and return false. As such, you don't even need an int oddCount, but a bool seenOdd. So instead of this:

int oddCount = 0;
//count the number of times a letter only appears once
for (int j = 0; j < 256; j++)
{
 if (alpha[j] % 2 == 1) oddCount++;
}
//if there is more than one letter that only occurs, then it 
//cannot be a palindrome.
return oddCount <= 1;

You could write:

bool seenOdd = false;
// scan for odd number of occurrences, stop after seeing two
for (int j = 0; j < 256; j++)
{
 if (alpha[j] % 2 == 1) {
 if (seenOdd) return false;
 seenOdd = true;
 }
}
// less then 2 letters with odd number of occurrences, must be true
return true;
added 848 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396
Loading
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396
Loading
lang-c

AltStyle によって変換されたページ (->オリジナル) /