-1

I have a small program that prints out the capital form of each letter of a word, but I get the error signed/unsigned mismatch when I compile it because I'm passing a cstring as a normal string in this program. How do I pass it correctly so that I can still use text.length()? Here is the error that I get "Tester.cpp(22,23): warning C4018: '<': signed/unsigned mismatch". It's at for (int i = 0; i < text.length(); i++)

#include <iostream>
using namespace std;

string capitalizeFirstLetter(string text);

int main() {
    char sentence[100];
    for ( ; ; )
    {
        cin.getline(sentence, 100);
        if (sentence != "0")
            capitalizeFirstLetter(sentence);
    }

    return 0;
}

string capitalizeFirstLetter(string text) {

    for (int i = 0; i < text.length(); i++)
    {
        if (i == 0)
        {
            text[i] = toupper(text[i]);
        }
        if (text[i] == ' ')
        {
            ++i;
            text[i] = toupper(text[i]);
        }
    }
    cout << text;

    return text;
}
Kuromiu
  • 31
  • 7
  • You can use `std::string::c_str()` but the length can be obtained through `strlen()` only in this case (or you pass it as parameter as well). Why don't you make `sentence` a `std::string` and use `std::getline(cin,sentence)` in 1st place? – πάντα ῥεῖ Sep 29 '20 at 06:49
  • Why not to use string::size_type as type of i instead of int? – Öö Tiib Sep 29 '20 at 06:51
  • The compiler warning means that you are using the wrong type for `i`, not that you are passing the string incorrectly. `i` should be `size_t` or (same thing) `string::size_type`. – john Sep 29 '20 at 07:34

4 Answers4

1

The simplest way to handle passing sentence as a string is to enclose it in a braced set, to provide direct initialization to the parameter std::string text eg..

    for ( ; ; )
    {
        std::cin.getline(sentence, 100);
        if (*sentence)
            capitalizeFirstLetter({sentence});
    }

This allows the character string sentence to be used as the Direct initialization to initialize std::string text in your capitalizeFirstLetter() function:

std::string capitalizeFirstLetter (std::string text) {

    for (size_t i = 0; i < text.length(); i++)
    {
        if (i == 0)
        {
            text[i] = toupper(text[i]);
        }
        if (text[i] == ' ')
        {
            ++i;
            text[i] = toupper(text[i]);
        }
    }
    std::cout << text;

    return text;
}

Your complete code, after reading Why is “using namespace std;” considered bad practice?, would then be:

#include <iostream>

std::string capitalizeFirstLetter (std::string text) {

    for (size_t i = 0; i < text.length(); i++)
    {
        if (i == 0)
        {
            text[i] = toupper(text[i]);
        }
        if (text[i] == ' ')
        {
            ++i;
            text[i] = toupper(text[i]);
        }
    }
    std::cout << text;

    return text;
}

int main (void) {
    
    char sentence[100];
    
    for ( ; ; )
    {
        std::cin.getline(sentence, 100);
        if (*sentence)
            capitalizeFirstLetter({sentence});
    }

    return 0;
}

(note: dereferencing sentence provides the first character which is then confirmed as something other than the nul-terminating character (ASCII 0))

A Better CapitalizeFirstLetter()

A slightly easier way to approach capitalization is to include <cctype> and an int to hold the last character read. Then the logic simply loops over each character and if the first character is an alpha-character, then capitalize it, otherwise only capitalize the letter when the current character is an alpha-character and the last character was whitespace, e.g.

std::string capitalizeFirstLetter (std::string text)
{
    int last = 0
    
    for (auto& c : text)
    {
        if (isalpha(c))
        {
            if (!i || isspace (last))
                c = toupper(c);
        }
        last = c;
    }
    std::cout << text;

    return text;
}

(note: the use of a range-based for loop above)

Either way works.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • I tried your code and it outputs the new string correctly. Thank you very much for the information and correction! Do you mind referring me to the use of ```:``` it's the first time for me to see it being used and I don't know its term. – Kuromiu Sep 29 '20 at 08:17
  • 1
    It is the [Range-based for loop (since C++11)](https://en.cppreference.com/w/cpp/language/range-for) a very handy shorthand for iterating over the elements of any container. – David C. Rankin Sep 29 '20 at 08:43
0

The error is not generating because of you passing a cstring as a normal string to the function but it is due to the fact that you are trying to compare c style string using != operator in the statement

if (sentence != "0")
    capitalizeFirstLetter(sentence);

try using strcmp() for that

Arsenic
  • 727
  • 1
  • 8
  • 22
  • 1
    I tried that, but the error appears at ```for (int i = 0; i < text.length(); i++)``` – Kuromiu Sep 29 '20 at 07:16
  • thats will be a warning which will hardly make any difference on output, but still if you want to remove it then you can type case **i** to **auto** – Arsenic Sep 29 '20 at 07:21
  • does that mean replacing every ```i``` with ```auto```? I tried that and it generated errors. I haven't encountered the use of ```auto``` in my lessons. – Kuromiu Sep 29 '20 at 07:37
  • no I mean changing data type to auto (learn about it here : https://www.geeksforgeeks.org/type-inference-in-c-auto-and-decltype/ ) – Arsenic Sep 29 '20 at 07:43
  • 1
    @Arsenic Using `auto` won't work. Even if you put `auto` before `i = 0`, it will take `i` as a signed integer. You need to either explicitly declare or make it unsigned during comparison (`; (unsigned) i < text.length();`). Although all these methods work, I would consider, declaring `i` as `std::string::size_type` is as the best option. Edit : Also, `auto i = 0ULL` will do the job. – brc-dd Sep 29 '20 at 07:50
0

Problems with your code :

  1. if (sentence != "0") : illegal comparison. If you want to break on getting 0 as input then try using strcmp (include <cstring>) as if (strcmp(sentence, "0"). (Note that strcmp returns 0 when two strings are equal.) Or simply do if (!(sentence[0] == '0' and sentence[1] == 0)). Moreover this condition should be accompanied with else break; to prevent the for loop from running forever.

  2. for (int i = 0; i < text.length(); i++) : generates warning because of comparison between signed and unsigned types. Change data-type of i to string::size_type to prevent the warning.

  3. <string> (for std::string) and <cctype> (for std::toupper) were not included.

  4. Thanks to @john for pointing this out. Your code has undefined behaviour if last character of a string is a space. Add a check if i is still less than text.length() or not before using text[i].

  5. Another case of error is when an space is there after 0. Move getline to condition of for to fix this. Now there will be no need to input a 0 to terminate program. Moreover, I recommend using while loop for this instead of for.

  6. You may also need to print a newline to separate sentences. Moreover, I would prefer printing the modified sentence in the main() function using the returned string from capitalizeFirstLetter.

  7. It doesn't matter much in short (beginner-level) codes, but avoid acquiring the habit of putting using namespace std; on the top of every code you write. Refer this.

Fixed code :

#include <cctype>
#include <cstring>
#include <iostream>
#include <string>
using namespace std;

string capitalizeFirstLetter(string text);

int main() {
  char sentence[100];
  while (cin.getline(sentence, 100))
    cout << capitalizeFirstLetter(sentence) << '\n';
}

string capitalizeFirstLetter(string text) {
  for (string::size_type i = 0; i < text.length(); i++) {
    if (i == 0)
      text[i] = toupper(text[i]);
    if (text[i] == ' ')
      if (++i < text.length())
        text[i] = toupper(text[i]);
  }
  return text;
}

Sample Run :

Input :

hello world
foo bar

Output :

Hello World
Foo Bar

My Version (Requires C++20) :

#include <cctype>
#include <iostream>
#include <string>

auto capitalizeFirstLetter(std::string text) {
  for (bool newWord = true; auto &&i : text) {
    i = newWord ? std::toupper(i) : i;
    newWord = std::isspace(i);
  }
  return text;
}

int main() {
  std::string sentence;
  while (std::getline(std::cin, sentence))
    std::cout << capitalizeFirstLetter(sentence) << std::endl;
}

Sample Run

brc-dd
  • 10,788
  • 3
  • 47
  • 67
  • First version will fail if a space is the last character in the string. – john Sep 29 '20 at 07:36
  • No I meant a space at the end of the string, e.g. `"hello "`. The first version of your code will then try to capitalize the character after the space, which of course doesn't exist. The OP's code has the same problem. – john Sep 29 '20 at 07:52
  • 1
    `text` is a `std::string`. I know that C++11 guarantees that a `\0` is stored, but isn't dereferencing to access that `\0` still a bounds error? Or are you now allowed to access `text[i]` where `i == text.size()`? – john Sep 29 '20 at 07:59
  • @john Yeah, I get it. You're right. `\0` will be stored past-the-end. Dereferencing it is undefined. [Ref.](https://stackoverflow.com/a/61669921/11613622) – brc-dd Sep 29 '20 at 08:03
0

Several things bugging me here.

First off, don't use using namespace std, it's "ok" in this case, but don't get used to it, it can cause quite some trouble. See Why is “using namespace std;” considered bad practice?

Next thing is, just use std::string instead of cstrings here, it's easier to write and to read and doesn't produce any measurable performance loss or something. And it's harder to produce bugs this way. So just use

std::string sentence;

and

getline(std::cin, sentence);

And why do you handle the output inside the function that transforms your string? Just let the main print the transformed string. So your main could look like this:

int main() {
    std::string sentence;
    
    while(true)
    {
        getline(std::cin, sentence);
        auto capitalized = capitalizeFirstLetter(sentence);
        std::cout << capitalized;
    }

    return 0;
}

PS: the 'error' you get is a warning, because you compare int i with text.length() which is of type size_t aka unsigned int or unsigned long int.

Stefan Riedel
  • 796
  • 4
  • 15
  • I see, that makes sense. I'm using ```using namespace std``` since I'm a beginner and I read from my book its use and other namespaces, which is highly required to explicitly add the prefix for readability and safety reasons, but I decided to not use ```std``` prefix since I'm not tackling namespace yet. – Kuromiu Sep 29 '20 at 08:11