vector in a struct doesn't work properly

Question

I declared a vector<string> and I cannot even compile it. I tried many ways but none of them worked. I'm trying to write out the x.surname.push_back(word)[i] but it's definetly written wrongly and I have no idea how to write it properly and make it possible to compile.

#include <cstring>
#include <iostream>
#include <vector>

using namespace std;

int main() {
  int number, i = 0;
  string word;
  struct donators {
    vector<string> surname;
    vector<int> amount;
  } x;

  cout << "How many donators do you want to register? " << endl;
  cin >> number;

  for (i = 0; i < number; i++) {
    cout << "Surname: ";
    cin >> word;
    x.surname.push_back(word)[i];

    cout << "Amount: ";
    x.amount.push_back(i);
    cin >> x.amount[i];
  }
  cout << "OUR GORGEUS DONATORS: " << endl;
  for (i = 0; i < number; i++) {

    if (x.amount[i] >= 10000) {
      cout << "Surname: " << x.surname(word)[i];
      cout << "Amount: " << x.amount[i] << endl;
    }

    else if (x.amount[i] < 10000) {
      cout << "Lack of surnames!" << endl;
    }
  }
  cout << "OUR CASUAL DONATORS: " << endl;

  for (i = 0; i < number; i++) {

    if (x.amount[i] < 10000) {
      cout << "Surname: " << x.surname(word)[i];
      cout << "Amount: " << x.amount[i] << endl;
    } else if (x.amount[i] >= 10000) {
      cout << "Lack of surnames!" << endl;
    }
  }

  return 0;
}

And one more thing. How to make sentence "Lack of surnames!" to be written out once? In some cases, it is written out twice or more times what is redundant.

If you get a compiler error, please add it to your question. — Max Langhof, Nov 15 '19 at 14:54
Why do you put `[i]` at the end of everything? What does that do/mean? Please explain what you wanted `x.surname.push_back(word)[i]` to do instead of what it does now. — Lightness Races in Orbit, Nov 15 '19 at 14:55
Your problem is that you think `std::vector` is working in a different way than `std::vector`. They both work the same and they should be used the same. You use `amount` (almost) correctly, use `surname` in the same way. — Yksisarvinen, Nov 15 '19 at 14:57
Please do yourself a favor and obtain [a good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). You will not learn C++ from trying random things (such as `x.surname(word)[i]`). Also, working iteratively would help: Your program has several syntax and logic errors. Don't try to write the entire program at once. Write things that work and expand them, fixing errors as soon as you notice them. Start with inputting and outputting a single name, then inputting and outputting a list of names, etc. — Max Langhof, Nov 15 '19 at 14:57
Better yet, you should make `Donator` struct only hold a single `std::string` and a single `int` and then use `std::vector` for everything. — Yksisarvinen, Nov 15 '19 at 14:58
[Don't use `using namespace std;`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). The code you write looks more like C then like C++. These are *not* the same languages. You should buy a good tutorial book on C++. — JHBonarius, Nov 15 '19 at 15:10
"_I cannot even compile it_" Then you can tell us what compiler you used, what command line you used, and what exact errors you got. [Edit] your post to include this essential information, instead of expecting readers to guess it psychically. — underscore_d, Nov 15 '19 at 15:50

score 0 · Answer 1 · answered Nov 15 '19 at 15:07

You are putting [i] at seemingly random places in your code. Such as in x.surname.push_back(word)[i];. Don't add things like this to your code if you're unsure about what they're doing.

The x.surname(word)[i] construct are also wrong. What's x.surname(word) supposed to be? This syntax is for function calls. surname, however, is not a function. It's a std::vector<std::string>. Just put x.surname[i] instead.

And one more thing. How to make sentence "Lack of surnames!" to be written out once? In some cases, it is written out twice or more times what is redundant.

That's because you write it for every donor that doesn't fit the criterion. Instead, keep track if any donor fits the criterion and only print it when none ends up fitting. You can do it like this:

bool HasGorgeousDonators = false;

And then in the loop:

if (x.amount[i] >= 10000)
{
    cout << "Surname: " << x.surname[i];
    cout << "Amount: " << x.amount[i] << endl;
    HasGorgeousDonators = true;
}

And after the loop:

if (!HasGorgeousDonators)
    cout << "Lack of surnames!" << endl;

Likewise for the other loop. Also, please consider the following Q&A:

Why is "using namespace std;" considered bad practice?

JHBonarius · Answer 2 · 2019-11-15T17:17:13.637

It seems like you are writing C with some C++ help functions. However C++ is a different language. Sure, it supports some C structures, but there's so much more. Take a look at some of my suggestions for implementation and compare it to your code:

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

template<typename T>
T ReadCin(std::string_view const& sv = "") {
    T retVal;
    if (!sv.empty()) std::cout << sv;
    std::cin >> retVal;
    return retVal;
}

class Donator {
private:
    std::string surname{};
    int amount{};
public:
    constexpr bool IsGenerous() const noexcept { return amount >= 10000; }
    void Read() noexcept {
        surname = ReadCin<decltype(surname)>("Surname: ");
        amount = ReadCin<decltype(amount)>("Amount: ");
    }
    friend std::ostream& operator<<(std::ostream& out, Donator const& donator) noexcept {
        out << "Surname: " << donator.surname << ", " << "Amount: " << donator.amount;
        return out;
    }
};

int main() {
    std::vector<Donator> donators(ReadCin<int>("How many donators do you want to register?\n"));
    for (auto& donator : donators) donator.Read();

    std::cout << "OUR GENEROUS DONATORS:\n";
    std::copy_if(std::cbegin(donators), std::cend(donators), std::ostream_iterator<Donator>(std::cout, "\n"),
        [](Donator const& donator) { return donator.IsGenerous(); });

    std::cout << "OUR CASUAL DONATORS:\n";
    for (auto const& donator : donators) if (!donator.IsGenerous()) std::cout << donator << '\n'; //alternative
}

I tried to include some of the possibilities using C++. I would really advise you to get a good book on C++.

vector in a struct doesn't work properly

2 Answers2