[etindoll]

I have written a program that counts the number of lowercase, uppercase, nubers, and other characters in a continuous string. The problem I am running into is when it counts through the string it returns the wrong count. I have already tried running each if and else if statement individually meaning I change the else to an if and only ran it by itself doing this they work fine as in they return the correct count. When I string them all together as below it will not do the correct count if I mix lowercase, uppercase, numbers, and other characters. I am at a lose to where to go from here.

#include <iostream>
using namespace std;

int main()
{

char s[50];
int i;
int lowercase = 0;
int uppercase = 0;
int numbers = 0;
int other = 0;
int total;

//User Prompted Input Information
cout << " Enter a contiuous string of characters with no blank spaces "<<endl;

cout << " (Example : Y0UrStr1nG%#&^) " <<endl << endl;

cout << " Enter your string: ";

cin >> s;

cout <<endl;

//loop through the string, counting numbers, letters & others

i = 0;

while (s[i] != 0)
{
if ((s[i] >= 'a' && s[i] <= 'z')){
   lowercase++; 
   i++;
}
else if ((s[i] >= 'A' && s[i] <= 'Z')){
   	uppercase++; 
	i++;
}	 
else if ((s[i] >= '0' && s[i] <= '9')){
	numbers++;
	i++;
}
else  
   other++;
   i++;
}

total = lowercase + uppercase + numbers + other;

cout << "Your string has " << lowercase << " lowercase letters." << endl;
cout << "Your string has " << uppercase << " uppercase letters." <<endl;
cout << "Your string has " << numbers << " numbers." <<endl;
cout << "Your string has " << other << " other characters." <<endl;
cout << "Your string has " << total << " total characters." <<endl;


system ("pause"); //pause for Dev-C++

return 0;
}

[Skydiver]

You shot yourself in the foot because of your inconsistent indent style. Notice that line 44 doesn't have an opening curly brace unlike the lines above it. It is uncaught because you changed styles on lines 30-31.

[etindoll]

I new to C++ programming and computer programming in general I have cleaned up my indent sytle. I tried the curly brace as suggested on line 44 and Dev-C++ give me this when I compile and execute Line 59 expected primary-expression before "else".

 #include <iostream>
using namespace std;

int main()
{

char s[50];
int i;
int lowercase = 0;
int uppercase = 0;
int numbers = 0;
int other = 0;
int total;

//User Prompted Input Information

cout << " Enter a contiuous string of characters with no blank spaces "<<endl;

cout << " (Example : Y0UrStr1nG%#&^) " <<endl << endl;

cout << " Enter your string: ";

cin >> s;

cout <<endl;

//loop through the string, counting numbers, letters & others

i = 0;

while (s[i] != 0)

{
if 
  ((s[i] >= 'a' && s[i] <= 'z'))
{
  lowercase++; 
  i++;
}

else 
   if 
      ((s[i] >= 'A' && s[i] <= 'Z'))
{
      uppercase++; 
      i++;
}
	 
else 
   if 
      ((s[i] >= '0' && s[i] <= '9'))
{
      numbers++;
      i++;
}

else  
   other++;
   i++;
}

total = lowercase + uppercase + numbers + other;

cout << "Your string has " << lowercase << " lowercase letters." << endl;

cout << "Your string has " << uppercase << " uppercase letters." <<endl;

cout << "Your string has " << numbers << " numbers." <<endl;

cout << "Your string has " << other << " other characters." <<endl;

cout << "Your string has " << total << " total characters." <<endl;


system ("pause"); //pause for Dev-C++

return 0;
}

[Adak]

Insert a { on line 58. Move the current line 58, down one line, (and all the other lines down one line as well.

[adgjlsfhk]

One other thing that I would do, but is not necessary is to make all of your numbers an array, and refer to them using an offset. This does not make the code more functional, but I at least find it easier to read.

[Skydiver]

Huh? Can you show an example?

To me the OP's code is very readable:

int lowercase = 0;
int uppercase = 0;
int numbers = 0;
int other = 0;
int total;
:
total = lowercase + uppercase + numbers + other;

This is unreadable using offsets:

int counts[5];
*(counts+4) = *(counts+0) + *(counts+1) + *(counts+2) + *(counts+3);

As well as this using indexes:

int counts[5];
counts[4] = counts[0] + counts[1] + counts[2] + counts[3];

Even using enums to try to make things more readable doesn't help:

enum { lowercase, uppercase, numbers, others, total, max };
int counts[max];
*(counts+total) = *(counts+lowercase) + *(counts+uppercase) + *(counts+numbers) + *(counts+others);

Or

enum { lowercase, uppercase, numbers, others, total, max };
int counts[max];
counts[total] = counts[lowercase] + counts[uppercase] + counts[numbers] + counts[others];

[adgjlsfhk]

I was thinking more like

int count[4]={0,0,0,0};
//lower, upper, number, other

then lower case is count[0]
upper case is count[1]
numbers are count[2]
and other is count[3]

[Skydiver]

And you'll end up with the following code to compute the total:

This is unreadable using offsets and magic numbers:

total = *(count + 0) + *(count + 1) + *(count + 2) + *(count + 3);

As well as this using indexes and magic numbers:

total = count[0] + count[1] + count[2] + count[3];

Even using enums to try to get rid of magic numbers doesn't help:

enum { lower, upper, number, other };
total = *(count + lower) + *(count + upper) + *(count + number) + *(count + other);

Or

enum { lower, upper, number, other };
total = count[lower] + count[upper] + count[number] + count[other];

[undefined behaviour]

Actually, I'm inclined to partially agree with adgjlsfhk. Not only would using arrays make this code shorter and more extendable, but also more portable. The code in the original post malfunctions when the execution character set is EBCDIC, because there are "other" characters between the values 'a' and 'z', and 'A' and 'Z'.

Skydiver, does the following code pass your legibility test? How would you improve it?

#include <cstdio>
#include <cstring>
#include <iostream>
#include <string>

struct search_t {
    char *description,
    size_t (*fn)(const char *, const char *),
    char *set,
    size_t count
};

int main(void) {
    search_t searches = { { "lowercase", strspn,  "abcdefghijklmnopqrstuvwxyz" },
                          { "uppercase", strspn,  "ABCDEFGHIJKLMNOPQRSTUVWXYZ" },
                          { "numeric",   strspn,  "0123456789" },
                          { "other",     strcspn, "abcdefghijklmnopqrstuvwxyz"
                                                  "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                                                  "0123456789" } };

    size_t total = 0;
    std::string str;

    std::cin >> str;
    while (str.c_str()[total] != '\0') {
        for (size_t search = 0; search < sizeof searches / sizeof *searches; search++) {
            size_t count = searches[search].fn(str.c_str() + total, searches[search].set);
            searches[search].count += count;
            total += count;
        }
    }

    for (size_t x = 0; x < sizeof searches / sizeof *searches; x++) {
        printf("Your string has %zu %s characters.\n", searches[search].count, searches[search].description);
    }
    std::cout << "Your string has " << total << " characters in total." << std::endl;

    return 0;
}

This post has been edited by undefined behaviour: 12 February 2013 - 07:51 AM

[Skydiver]

Ah, much more readable except for the duplication of str.c_str(). Perhaps a simple const char *current = str.c_str() between lines 24-25 would be useful.

There is an interesting bug, though with the assumption that the character sets are mutually exclusive. So if I wanted to add the following between lines 16 and 17, I would be shooting myself in the foot:

{ "alphanumeric", strspn,  "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" },

[adgjlsfhk]

you could also just add another column for total,

int count[5]={0,0,0,0,0}
//same but total at the end

then you could add 1 to count[5] at the end of the loop.