3

I am creating a digital vending machine of sorts for school and have run into an issue. I created a struct of Items for vending. I then created a struct called Machine that contains a vector<Items>. I wanted to create a for loop that iterates over the vector<Item> and displays the items but I get the following error:

C:\Users\Nate\Desktop>g++ structversion.cpp -o structversion.exe -std=c++11
structversion.cpp: In function 'int test(Machine)':
structversion.cpp:29:20: error: 'begin' was not declared in this scope
   for (Item item : machine) {
                    ^
structversion.cpp:29:20: note: suggested alternatives:
In file included from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/string:51:0,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/locale_classes.h:40,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/ios_base.h:41,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ios:42,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ostream:38,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/iostream:39,
                 from structversion.cpp:1:
C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/range_access.h:87:5: note:   'std::begin'
     begin(_Tp (&__arr)[_Nm])
     ^
C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/range_access.h:87:5: note:   'std::begin'
structversion.cpp:29:20: error: 'end' was not declared in this scope
   for (Item item : machine) {
                    ^
structversion.cpp:29:20: note: suggested alternatives:
In file included from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/string:51:0,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/locale_classes.h:40,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/ios_base.h:41,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ios:42,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ostream:38,
                 from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/iostream:39,
                 from structversion.cpp:1:
C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/range_access.h:97:5: note:   'std::end'
     end(_Tp (&__arr)[_Nm])
     ^
C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/range_access.h:97:5: note:   'std::end'

I apologize if this is a redundant or dumb question. Also here is the code at issue:

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

struct Item 
{
    string name;
    double price;
    unsigned int quantity;
    unsigned int amountInCart;
    bool addedToCart;
};

struct Machine { vector<Item> menu; };

void initItem(Item& i, string name, double price, unsigned int quantity,
    unsigned int amountInCart, bool addedToCart) 
{
    i.name = name;
    i.price = price;
    i.quantity = quantity;
    i.amountInCart = amountInCart;
    i.addedToCart = addedToCart;
}

test(Machine machine)
{
    for (Item i : machine) {
        cout << "item = " << i.name;
    }
}

main()
{
    Item cake;
    Item fruit;
    Item chips;
    Item soda;
    Item juice;

    initItem(cake, "Cake", 3.00, 5, 0, false);
    initItem(fruit, "Fruit", 4.20, 15, 0, false);
    initItem(chips, "Chips", 1.00, 6, 0, false);
    initItem(soda, "Soda", 1.50, 7, 0, false);
    initItem(juice, "Juice", 1.90, 10, 0, false);

    Machine machine;
    machine.menu.push_back(cake);
    machine.menu.push_back(fruit);
    machine.menu.push_back(chips);
    machine.menu.push_back(soda);
    machine.menu.push_back(juice);

    test(machine);
    return 0;
}

The function test is where I am trying to iterate over the vector menu within the Machine struct.

I am fairly new so if anyone has time and could ELI5 what I am doing wrong that would be amazing.

JeJo
  • 30,635
  • 6
  • 49
  • 88
cappyt
  • 51
  • 1
  • Hope that is a copy-paste typo: your `test` and `main` missed the returns. – JeJo Sep 28 '19 at 06:29
  • oh yes it was! This was just a snippet I took of the broken code from my main project which is bigger. Just wanted to only include the relevant code. Once again thanks for the assist. – cappyt Sep 28 '19 at 06:52
  • You should learn what a constructor is, please do not use obscure init functions! Also you can use aggregate initialization in your case here. – Klaus Sep 28 '19 at 07:22

1 Answers1

5

In order to use the range-based for loop in your user-defined types, you need to define begin() and end() iterators. That means, Machine needs to have the following member functions which return the iterator of the member container. (See live online)

struct Machine
{
    vector<Item> menu;
    auto begin() { return menu.begin(); } // auto return requires C++14
    auto end()   { return menu.end();   }

    // or in C++11, you provide the correct return type
    // decltype(menu.begin()) begin() { return menu.begin(); }
    // decltype(menu.end())   end()   { return menu.end();   }
};

Otherwise, you need to provide the iterative object directly to the loop. That means, in your case, in order to work with a minimal change:

void test(Machine const& machine) // pass by `const-ref` as the machine is read-only inside the function
{
    for (const Item& i : machine.menu) { // const& : same reason as above ^^
    //                   ^^^^^^^^^^^^ --> std::vector has iterators: begin(), end() 
        cout << "item = " << i.name;
    }
}

Some other notes:

  • The initItem function is not the way you do in normally in C++ (or any other object-oriented programming languages). That is the job of a constructor. In order to change the value of the member(s) in an object, the setter (member) function will be used. A good starting would be:

    class Item
    {
    private:
        std::string name;
        double price;
        unsigned int quantity;
        unsigned int amountInCart;
        bool addedToCart;
    
    public:
        // constructor
        Item(std::string name, double price, unsigned int quantity,
            unsigned int amountInCart, bool addedToCart)
            : name{ name } // or name{ std::move(name) } when you learn about the move-semantics
            , price{ price}
            , quantity{ quantity }
            , amountInCart{ amountInCart }
            , addedToCart{ addedToCart }
        {}
        // ... getter and setter functions
    };
    

    You create an object now like:

    Item cake{ "Cake", 3.00, 5, 0, false };
    
  • Please do not practice with using namespace std;. See here for more: Why is "using namespace std;" considered bad practice?
  • I have passed the Machine object to the function test(also in range based loop, Item objects) passed by const qualified referance. Whenver, the data is non-modifiable under the any scopes, you should pass the parameters like that, so that unwanted copying can be avoided(creadits @Klaus): . For further readings: Is it better in C++ to pass by value or pass by constant reference?
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • Oh wow I see. That is very simple. Thank you very much for the quick response and your help! – cappyt Sep 28 '19 at 06:27
  • You should auto& instead of auto please! If not, you take a copy of every string you iterate over! – Klaus Sep 28 '19 at 07:24
  • @Klaus Oh ja.. for sure. Added in the answer. Thanks for pointing out. – JeJo Sep 28 '19 at 07:28
  • Maybe you can add some hint *why* you use ref. You pointed out the "read only" which did not describe the "need" for ref here. I only mention this as the original code from OP is far away from c++ so it is maybe an idea to help around all that important c++ features here and make your answer more helpful. BTW: There is no need for a constructor as you can use aggregate initialization and an = default will also be enough. OK, it grows and grows :-) – Klaus Sep 28 '19 at 07:32
  • 1
    @Klaus Adding more helpful notes: of course good, but I wanted to explain the problem what OP really had in the code. The code improvements should have been always unrelated comments, not in the answer, unless it demands. I have updated anyway including a further reading. – JeJo Sep 28 '19 at 07:48