patterncMinor
Ask for stream of character input and print number of uppercase/lowercase characters
Viewed 0 times
streamnumberlowercaseuppercaseaskcharacterinputprintcharactersfor
Problem
Does this code look okay?
#include
#include
int case_letters();
int main(void)
{
case_letters();
return 0;
}
int case_letters()
{
int ch;
int numOfUpper = 0;
int numOfLower = 0;
printf("please enter a some characters, and ctrl + d to see result\n");
while ((ch = getchar()) != EOF)
{
if (isdigit(ch))
{
printf("please enter a valid character\n");
continue;
}
else if (isupper(ch))
{
numOfUpper++;
}
else if (islower(ch))
{
numOfLower++;
}
}
return printf("There are %d Uppercase letters, and %d Lowercase letters\n", numOfUpper, numOfLower);
}Solution
-
It looks unusual to have newlines between the function statement and body since they're associated with each other. This just adds extra lines and makes the code a little harder to read.
-
If
-
Unformatted outputs should use
-
As @Rhs has mentioned, your program doesn't handle special characters. Based on this from @Loki Astari, there are additional types of characters and with accompanying testing functions.
-
According to this answer, CTRLD only works on Mac OS and Linux. For Windows, it's CTRLZ. All of this should be stated in the output in case the user is running this on Windows.
-
Consider making this more modular for increased maintainability:
Here's what I came up with:
It looks unusual to have newlines between the function statement and body since they're associated with each other. This just adds extra lines and makes the code a little harder to read.
-
If
case_letters() is just supposed to print something at the end and not return a value, it should return void (no value), not int. main() isn't doing anything with this function after calling it.-
Unformatted outputs should use
puts() instead of printf().-
As @Rhs has mentioned, your program doesn't handle special characters. Based on this from @Loki Astari, there are additional types of characters and with accompanying testing functions.
-
According to this answer, CTRLD only works on Mac OS and Linux. For Windows, it's CTRLZ. All of this should be stated in the output in case the user is running this on Windows.
-
Consider making this more modular for increased maintainability:
- keep letter counters for final output (in
main())
- collect each character from user
- determine if the character is a letter
- increment either counter if a letter is entered
Here's what I came up with:
#include
#include // add this library
#include
bool valid_char(char ch)
{
if (isdigit(ch)) return false;
// include additional non-letter checks...
// if everything checks out:
return true;
}
void update_counters(char ch, int* numOfUpper, int* numOfLower)
{
if (isupper(ch))
{
(*numOfUpper)++;
}
else if (islower(ch))
{
(*numOfLower)++;
}
}
void get_letters(int* numOfUpper, int* numOfLower)
{
puts("Please enter some characters\n");
puts("Press CTRL+D on Mac OS/Linux or CTRL+Z on Windows to see the results\n");
char ch;
while ((ch = getchar()) != EOF)
{
while (!valid_char(ch))
{
puts("Character must be a letter\n");
ch = getchar();
}
update_counters(ch, numOfUpper, numOfLower);
}
}
int main(void)
{
int numOfUpper = 0;
int numOfLower = 0;
int* upperPtr = &numOfUpper;
int* lowerPtr = &numOfLower;
get_letters(upperPtr, lowerPtr);
printf("There are %d uppercase letters and %d lowercase letters\n", numOfUpper, numOfLower);
}Code Snippets
#include <ctype.h>
#include <stdbool.h> // add this library
#include <stdio.h>
bool valid_char(char ch)
{
if (isdigit(ch)) return false;
// include additional non-letter checks...
// if everything checks out:
return true;
}
void update_counters(char ch, int* numOfUpper, int* numOfLower)
{
if (isupper(ch))
{
(*numOfUpper)++;
}
else if (islower(ch))
{
(*numOfLower)++;
}
}
void get_letters(int* numOfUpper, int* numOfLower)
{
puts("Please enter some characters\n");
puts("Press CTRL+D on Mac OS/Linux or CTRL+Z on Windows to see the results\n");
char ch;
while ((ch = getchar()) != EOF)
{
while (!valid_char(ch))
{
puts("Character must be a letter\n");
ch = getchar();
}
update_counters(ch, numOfUpper, numOfLower);
}
}
int main(void)
{
int numOfUpper = 0;
int numOfLower = 0;
int* upperPtr = &numOfUpper;
int* lowerPtr = &numOfLower;
get_letters(upperPtr, lowerPtr);
printf("There are %d uppercase letters and %d lowercase letters\n", numOfUpper, numOfLower);
}Context
StackExchange Code Review Q#21182, answer score: 6
Revisions (0)
No revisions yet.