Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

(Actually, as @200_success points out you don't need this corner case. I'm going to leave this up here though, because sometimes you will get cases like this and you should consider reworking them if they appear awkward)

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

(Actually, as @200_success points out you don't need this corner case. I'm going to leave this up here though, because sometimes you will get cases like this and you should consider reworking them if they appear awkward)

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

(Actually, as @200_success points out you don't need this corner case. I'm going to leave this up here though, because sometimes you will get cases like this and you should consider reworking them if they appear awkward)

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

added 226 characters in body
Source Link
Dair
  • 6.2k
  • 1
  • 21
  • 45

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

(Actually, as @200_success points out you don't need this corner case. I'm going to leave this up here though, because sometimes you will get cases like this and you should consider reworking them if they appear awkward)

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

(Actually, as @200_success points out you don't need this corner case. I'm going to leave this up here though, because sometimes you will get cases like this and you should consider reworking them if they appear awkward)

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

Source Link
Dair
  • 6.2k
  • 1
  • 21
  • 45

isspace

There are other characters besides for spaces. What happens if I do:

Hello<tab><tab>world!

Your code will report that there is one word. I would rewrite these:

if(*string==' '){
 //get previous character
 string--;
 // If previous character is not a space we increase the count
 // Otherwise we dont since we already counted a word
 if(*string!=' '){
 count++;
 }

You should instead use isspace for these kind of things. ' ' is for explicitly a space.

Indentation

Fix your indentation. Your main uses 4 spaces, your word_counter uses (maybe?) 2. Be sure that it is consistent. Choose one or the other.

Empty case rework

Your empty corner case can be reworked:

 int count;
 if(*string!='0円'){
 count=1;
}
 else{
 count=0;
 return 0;
 }

First you don't need to set count = 0 if you just return immediately. I would restructure you if statement to be:

if (*string == '0円') {
 return 0;
}

And from there continue with:

int count = 1;

This means we don't leave count uninitialized either.

lang-c

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