1

Here is a program in C++ for a string that accepts all characters but only outputs the letters; if letters are lower case then we should make them upper case.

#include<iostream> #include<cstdlib> #include<cctype> #include <iomanip> #include <cstring> using std :: cin; using std :: cout; using std :: endl; using std::setw ; const int MAX_STR_LEN=100; int main() { char str1 [MAX_STR_LEN]; int i; cin >> setw(MAX_STR_LEN) >> str1; for (int i=0; i <MAX_STR_LEN; i++) { if (isalpha(str1[i])) { if (str1[i]>='A'&& str1[i]<= 'Z') cout << str1[i]; if (str1[i]>='a'&& str1[i]<= 'z') { str1[i]= toupper(str1 [i]); cout << str1[i]; } } } return EXIT_SUCCESS; } 

This tends to work fine but gives extra letters, seems like I'm overlooking something. Also, when I only input numbers it gives letters such as PHUYXPU, something that I didn't input.

3
  • 3
    What if the actual input was less than MAX_STR_LEN? You forgot to check for the terminating '\0' character. Commented Jan 16, 2016 at 10:40
  • why not use std::string? Commented Jan 16, 2016 at 12:00
  • Unrelated to your current problem, but isalpha and toupper don't take chars. They take ints that hold the values of unsigned char (or EOF, but that's not possible here). Call isalpha((unsigned char) str1[i]) instead. Plenty of systems make isalpha(str1[i]) do the same thing, but on some, if str1[i] < 0, isalpha(str1[i]) can misbehave quite badly, for example by crashing your whole program. Commented Jan 16, 2016 at 13:30

3 Answers 3

2
for (int i=0; i <MAX_STR_LEN; i++) 

This would mean that you would iterate all the 100 cells of the array irrespective of the length of the string. You should either initialise the array before the cin statement like:

for(i=0;i<MAX_STR_LEN;i++) str1[i] = '\0'; 

or replace the condition in the for loop to iterate only through the length of the array like:

for(int i=0;i<strlen(str1);i++) { //blah blah blah 
Sign up to request clarification or add additional context in comments.

2 Comments

Don't call strlen(str1) in a loop though. There's no point in computing the length again for every character, it's not going to change.
Much better condition is str[i] != '\0', or shorter str[i].
2

In C++, you can do a lot without a raw loops, try this:

#include <iostream> #include <string> #include <algorithm> using namespace std; struct to_upper { char operator()(char c) { return toupper((unsigned char)c); } }; int main(int argc, const char * argv[]) { string letters, input; cout << "Enter your name: "; getline(cin, input); copy_if(begin(input), end(input), back_inserter(letters), ::isalpha); transform(begin(letters), end(letters), begin(letters), to_upper()); cout << letters << endl; } 

Result:

Enter your name: m1ar7ko0
MARKO

2 Comments

One problem in the OP's code is missing is a cast to unsigned char before calling isalpha. This is also a problem in your version, except made worse because there's no longer a simple way to add that cast.
You are right, thanks. He can use functor or lambda.
-1

If i got it right you don't need the isalpha() function, you are already removing the numeric and symbols with the subsequent if (str1[i]>='A'&& str1[i]<= 'Z') and if (str1[i]>='a'&& str1[i]<= 'z').

1 Comment

i tried that, it didnt work. it gave extra letters, and when i only input numbers it gave me letters. my point was to try to be specific as possible to diminish any possibility of invalid output

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.