-2

I have problem with this simple code. The program counts the letters in the string / vector.

I have two methods, one for vector and one for string. In main(), on the first line, I want to use the method for vector in first parameter, and on the second line I want to use the method for string. But both commands are using the string method.

#include <iostream>
#include <vector>
using namespace std;

int How_many_vector=0;
int How_many_string=0;

int count_chars(vector<char>& word, char character){
    for(int i = 0; i <=word.size();i++){
        if(word[i]==character){
            How_many_vector++;
        }
    }

    return How_many_vector;
}

int count_chars(string wordstring,char character){
    for(int i = 0;i<=wordstring.length();i++){
        if (wordstring[i]==character){
            How_many_string++;
        }
    }
    return How_many_string;
}

int main(){
    cout<< count_chars({'t','a','b','t','z'},'t')<<endl;
    cout<<count_chars("balcer",'b');
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • There's multiple bad things about your code: 1. you're using global variables (`How_may_vector`) where you **really** want a variable within the function. 2. there's functions that already do what you want in `std::`. 3. Never use `using namespace std;`. 4. this mixes C++ programming style from 25 years ago with relatively modern initialization methods. This feels like you're not really trying to understand things, but are just copy and pasting stuff. 5. use an IDE that has a "format my code" button. Your code isn't well-readable like it is, and that makes *your* life harder! – Marcus Müller Oct 14 '21 at 23:48
  • 1
    6. is `{'t','a','b','t','z'}` an initializer for a `string` or a `vector`? Practically impossible to tell as a human... – Marcus Müller Oct 14 '21 at 23:49
  • sorry but this is a project for my studies and i have to write it as simple as possible. That's what they teach in college, they told me not to look at things like "using namespace std;" – fresco357 Oct 14 '21 at 23:51
  • Appending to @MarcusMüller : Finally if you are counting negative values are meaningless anyway (unless you wanted to use them to encode errors), so more suitable return type would be `size_t`... – Aconcagua Oct 14 '21 at 23:51
  • this is anything but "as simple as possible". It's complicated, and it actually is functionally *wrong*. The globals here **make your code break**. This is 0 points if you handed it in. You need to learn at least basic C++. – Marcus Müller Oct 14 '21 at 23:52
  • {'t','a','b','t','z'} is for vector and "balcer" is for string – fresco357 Oct 14 '21 at 23:52
  • 1
    Note: `<=` in a `for` loop iterating over a container is almost always wrong. You should doublecheck to ensure the program is not reading out-of-bounds. – user4581301 Oct 14 '21 at 23:53
  • 1
    @fresco357 your compiler might disagree with you. `{'t','a','b','t','z'};` is a perfectly valid way to initialize a string: `string s{'t','a','b','t','z'};`! – Marcus Müller Oct 14 '21 at 23:54
  • @user4581301 Well, at least for the string there would be the terminating null character... But agree. – Aconcagua Oct 14 '21 at 23:55
  • your whole "simple as possible" code could be reduced to one include (`#include `), and a call to a function that is called **exactly** like you'd expect it to be called. – Marcus Müller Oct 14 '21 at 23:55
  • @Aconcagua no, a `std::string` is not a zero-terminated C string. – Marcus Müller Oct 14 '21 at 23:56
  • chillout, I started writing c ++ an hour ago – fresco357 Oct 14 '21 at 23:57
  • @fresco357 congrats :) but seriously, this is bad code, and you need to understand why you can't use these globals (and why they are inappropriate anyways). Assume you call the same function twice, what is your result? – Marcus Müller Oct 14 '21 at 23:58
  • @MarcusMüller Not any more since [C++11](https://en.cppreference.com/w/cpp/string/basic_string/operator_at)... – Aconcagua Oct 14 '21 at 23:59
  • @Aconcagua you're misinterpreting that! *The last element of a `std::string` is not a zero byte*! The `[]` operator pointed one beyond the last element will return 0; that's two different things. – Marcus Müller Oct 15 '21 at 00:00
  • Is this supposed to be a first assignment? There's a lot of principles at play here that I wouldn't expect in a first assignment. But as others have pointed out, it's likely that this is just a copy/pasta franken-program. I mean, if you have a vector version and aren't just immediately returning with the vector's built-in size function, you're doing it wrong. – sweenish Oct 15 '21 at 00:03
  • 2
    @MarcusMüller Well, we *do* get a null character, and [`data` (or `c_str`)](https://en.cppreference.com/w/cpp/string/basic_string/data) returns a null terminated array – even in its const version. If now the character is there *internally* or not (in which case we'd need a mutable internal representation) doesn't really matter, the string at least behaves as if. My original (humoristically meant) intention was to hint to that with `std::string` we *can* include `i == length()` in the loop, which remains valid in any case ;) – Aconcagua Oct 15 '21 at 00:15
  • :D that's quite true, and the `data() const` argument shows you're probably right implying the underlying storage has to be zero-terminated. – Marcus Müller Oct 15 '21 at 00:16
  • By the way, **you forgot to ask a question**, and this is a question & answer site. Formulating a clear question is really an important step in getting better at solving problems! – Marcus Müller Oct 15 '21 at 00:24
  • Ignoring everything else, it is nice to see `vector` and `string` allowed in an early assignment. – user4581301 Oct 15 '21 at 00:38

2 Answers2

2

Both std::vector and std::string can be constructed from a brace list. So you have to be more explicit about which function you actually want to call when passing it a brace list.

You can't bind a vector& reference to a brace list. And you can't bind it to a temporary vector, either. So the 1st line can't call the vector version of the function.

However, a string object can be constructed from a brace list, and the string version of the function takes a string by value, not by reference. So, the 1st line can (and does) call the string version of the function, by creating a temporary string from the brace list, and then passing that temporary to the function.

If you want to call the vector version instead, you will have to call it with a pre-existing vector object for the vector& reference to bind to:

#include <iostream>
#include <vector>
#include <string>
using namespace std;

int count_chars(vector<char>& word, char character){
    int How_many = 0;
    for(size_t i = 0; i < word.size(); ++i){
        if (word[i] == character){
            ++How_many;
        }
    }
    return How_many;
}

int count_chars(string wordstring, char character){
    int How_many = 0;
    for(size_t i = 0; i < wordstring.length(); ++i){
        if (wordstring[i] == character){
            ++How_many;
        }
    }
    return How_many;
}

int main(){
    vector<char> vec{'t','a','b','t','z'};
    cout << count_chars(vec, 't') << endl;
    cout << count_chars("balcer", 'b');
}

If you don't want to declare a separate variable, then change the vector& reference to const vector& instead (which you should do anyway, since the function is just reading values, not modifying them), and then you can construct a temporary vector object directly in the parameter for it to bind to (as a const reference can bind to a temporary):

#include <iostream>
#include <vector>
#include <string>
using namespace std;

int count_chars(const vector<char>& word, char character){
    int How_many = 0;
    for(size_t i = 0; i < word.size(); ++i){
        if (word[i] == character){
            ++How_many;
        }
    }
    return How_many;
}

int count_chars(const string &wordstring, char character){
    int How_many = 0;
    for(int i = 0; i < wordstring.length(); ++i){
        if (wordstring[i] == character){
            ++How_many;
        }
    }
    return How_many;
}

int main(){
    cout << count_chars(vector<char>{'t','a','b','t','z'}, 't') << endl;
    cout << count_chars("balcer", 'b');
}

And just FYI, both of your functions are redundant, as the standard library has a std::count() algorithm for counting the number of occurrences of a given value in a range of values, such as vectors and strings.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • And thats what im looking for, thanku you very much for help – fresco357 Oct 15 '21 at 00:11
  • pretty involved answer for someone who's an hour on C++ – @fresco357 make sure you really understand what a reference is ;) Remy, that's a nice answer – upvoted. – Marcus Müller Oct 15 '21 at 00:17
  • (I wish I could give extra points for replacing the `for(int` loops with `for(const char c: wordstring) { How_many += (c==character); }`) – Marcus Müller Oct 15 '21 at 00:21
  • @fresco357 Note that actually `wordstring` should be passed as const reference as well to avoid an unnecessary copy. – Aconcagua Oct 15 '21 at 00:21
  • c ++ is much more difficult to start with than java :/ – fresco357 Oct 15 '21 at 00:21
  • 1
    @fresco357 what did you expect? C++ is one of the most complex, flexible, low-level languages around. It is going to take you a long time to really understand its features and its quirks. I suggest you get yourself some [good books](https://stackoverflow.com/questions/388242/) to study from. – Remy Lebeau Oct 15 '21 at 00:22
  • @fresco357 honestly, it depends on the literature, and how well-guided you approach it. Your code definitely is copied together - you're using *references* before you even understood what a scope is (which, by the way, works exactly as in Java, so your globals would be a sin in Java, just as well – you can't have learned a lot of Java yet!). Copying together code is of course not going to teach you in a logical way how to solve simple problems before approaching more complex ones. – Marcus Müller Oct 15 '21 at 00:23
  • Man, I am still finding and cleaning up pseudo C++ code I wrote back in 2008-2010 when I figured years of experience in C and Java was going to make me a whiz at C++. I find it and I cringe. Sometimes I see stuff so dumb that I give thanks to an assortment of deities that I didn't get anyone killed – user4581301 Oct 15 '21 at 00:45
0

"simple as possible" is quite simple, you do that by using library functions. By the way, code on this site is licensed cc-by-sa, so you need to cite me when you hand this in:

#include <algorithm>
#include <iostream>
#include <string>
#include <vector>

using std::count;

int main() {
    std::vector<char> v{'t', 'a', 'b', 't', 'z'};
    std::string s("balcer");

    std::cout << count(v.begin(), v.end(), 't') << "\n"
              << count(s.begin(), s.end(), 'b') << "\n";
}
Marcus Müller
  • 34,677
  • 4
  • 53
  • 94