-1

I am trying to write a function that counts the number of characters in a string excluding spaces. However, the output is always wrong so there is something wrong with my code.

There is something wrong with my code but I can't figure it out. Please help me with my programming homework.

#include <iostream>
#include <bits/stdc++.h>
#include <stdio.h>
#include <ctype.h>
using namespace std; 

int countLetters (char s[], int size_s){
    int isLetter = 0;
    for(int i =0; i<size_s;i++){
        if (isalpha(s[i])){
            isLetter ++;
        }
    }
    return isLetter; 
}

int main (){
    char s[100]; 
    gets(s); 
    int n = sizeof(s)/sizeof(s[0]);
    cout << countLetters(s,n);

}

Here is an example of the wrong output:

hi
10
PS C:\Users\user\OneDrive\Desktop\cpp practical> cd "c:\Users\user\OneDrive\Desktop\cpp practical\" ; if ($?) 
{ g++ count_letters.cpp -o count_letters } ; if ($?) { .\count_letters }
hi
6
PS C:\Users\user\OneDrive\Desktop\cpp practical> cd "c:\Users\user\OneDrive\Desktop\cpp practical\" ; if ($?) 
{ g++ count_letters.cpp -o count_letters } ; if ($?) { .\count_letters }
hi
10
PS C:\Users\user\OneDrive\Desktop\cpp practical>
JaMiT
  • 14,422
  • 4
  • 15
  • 31
  • 1
    Side notes: (1) `#include `: https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h, (2) `using namespace std;`: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice – wohlstad Jan 28 '23 at 06:54
  • *"the output is always wrong"* -- could you give an example of the wrong output? Looking for patterns in what you get can be as valuable a debugging tool as staring at code over and over again. Another potentially useful debugging tool that you could work into your question is a code walkthrough. Assume the input is simply "A". Walk us through what you expect your code to do with this input. – JaMiT Jan 28 '23 at 06:57
  • 2
    FYI -- `return std::count_if(s, s + size_s, [](char c) { return isalpha(c); });` – PaulMcKenzie Jan 28 '23 at 06:58
  • 4
    Where are you learning C++ from? It seems a low quality or outdated source. You don't need "C" style arrays for managing strings. There is std::string and/or std::string_view for those. Code with sizeof(s)/sizeof(t) should not be (almost never) necessary in C++. Including any header from the C++ library with .h (like #include /) is an indication you are trying to use code that's mostly there for backward compatibility with "C". Try to avoid these too – Pepijn Kramer Jan 28 '23 at 07:05
  • Aren't you simply overshooting through the string length by reading 100 characters of uninitialized data? **What is the value of `n`? And what is the string length you expect to test?** – Atmo Jan 28 '23 at 07:07
  • 2
    I absolutely support and upvoted the comment posted by @PepijnKramer. I actually had to check about the `gets` function [here](https://www.programiz.com/cpp-programming/library-function/cstdio/gets). Deprecated since C++11, removed since C++14.... no wonder I did not know about it. – Atmo Jan 28 '23 at 07:16
  • To already help OP a bit : https://en.cppreference.com/w/ is the site with most up-to-date reference material. Then there is this [book list](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). And to get going : https://www.learncpp.com/ (it is very decent, and the minor issues it has you can easily improve on later). After you've learned some C++ have a look at the [C++ core guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) to double check if you are still using the latests insights into how to use C++ – Pepijn Kramer Jan 28 '23 at 07:21
  • `int n = sizeof(s)/sizeof(s[0]);` This is the size of the array, not the length of the string. Try `int n = strlen(s);`. If you had added some code `cout << n << endl;` you could have easily worked this out for yourself. – john Jan 28 '23 at 07:46
  • there is no reason anymore to use `sizeof(s)/sizeof(s[0])` to get size of an array ever. For c-strings it never was the way, and for c-arrays you can use `std::size`. `std::string` has a `size` method too – 463035818_is_not_an_ai Jan 28 '23 at 09:28
  • Adding output is good. (It could have been presented better, though. It might be clearer to write *"I entered `hi` and the output was sometimes `10` and sometimes `6`. I expected `2`."*) Strange that you counted more characters than you entered, isn't it? My inclination would be to check your bounds -- what is the value of `n` right before you pass it to `countLetters()`? – JaMiT Jan 29 '23 at 03:54

1 Answers1

0

To begin with, I think your information source, like PepijnKramer, said in the comments, is a bit old. There are much easier data types you can use to complete this problem like std::string below.

First of all, your code is redundant in its header files: #include <bits/stdc++.h> is a nonstandard header, and you already have all the libraries you need.

Secondly, your main problem comes from the lines

char s[100];
gets(s);
int n = sizeof(s)/sizeof(s[0]);

You see, you first create a char array called s with 100 elements. However, you never give them an initial value, so the random garbage that came with the initialization stays. However, C++ also does not reset the values, so some of them may be randomized into letters. Then, you just use gets() to input the string. However, gets() only fills the array s with the number of characters you entered. Therefore, if you do not enter 100 characters, the random garbage that still can be a letter remains. Finally, when you do the size with the integer n, it turns out the result is always 100 because you defined s to have 100 elements, so when you run the function, you still iterate over the garbage in the array s. Therefore, depending on chance of what the garbage includes, your output is incorrect. To solve this, you can either you make sure to initialize it by

char s[100] = { };

Or switch a datatype. Here is a correct solution using std::string:

#include <bits/stdc++.h>
using namespace std; 

int countLetters (string a, int size_s){
    int isLetter = 0;
    for (int i = 0; i < size_s; i++) {
        if (isalpha(a[i])){
            isLetter++;
        }
    }
    return isLetter; 
}

int main () {
    string s; 
    getline(cin, s); 
    cout << countLetters(s, s.length());
}
Hudson
  • 312
  • 2
  • 18
  • *"`#include ` already contains the other libraries you attached."* -- only if using a toolchain that supplies `bits/stdc++.h`. This is a non-standard header. Instead of removing the other headers, you should have removed this one. Also see [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) – JaMiT Feb 14 '23 at 03:50
  • One-and-a-halfth option: use `strlen()`. – Dúthomhas Feb 16 '23 at 02:30