1

I am making a sudoku game in C++ to improve and learn SFML more thoughtfully, but I have run into a problem with range based loops.

I have an array of Tables like this:

Table Tables[9];

and when I loop through them like so:

for(auto table: Tables){
    //do stuff
   }

There is no problem. Every table has an array of sub-fields, like so

Field fields[9];

which are supposed to be accessed with getter

Field *Table::getFields() {
    return fields;
}

But when I try to go through it all with

for(auto table: Tables){
    for(auto field: table.getFields()) {
        //do stuff
    }
}

I get an error "Field *" is not a valid range type. How do I return the array so it's loop-able?

Also: I have tested Tables[i].getFields()[f].doStuff and it work just fine, so I believe the array is returned.

user0042
  • 7,691
  • 3
  • 20
  • 37
Metju.Perry
  • 61
  • 1
  • 7
  • 6
    You use `std::array` instead of C arrays. `Field *` is a pointer and has lost the `9` so there is no information about the size. – nwp Oct 12 '17 at 15:49
  • Possible duplicate of [What is array decaying?](https://stackoverflow.com/questions/1461432/what-is-array-decaying) – Passer By Oct 12 '17 at 15:53
  • While I can't confirm since you're not showing us all your code, you're probably running afoul of undefined behavior as well as you seem to be returning the address of a temporary variable on the stack. – Mgetz Oct 12 '17 at 15:58
  • @Mgetz I assumed that was the address of a member. Which probably shouldn’t have a getter at all and just be `public`, but without all the code it’s hard to tell. – Daniel H Oct 12 '17 at 16:00
  • @DanielH I've long since learned not to assume in cases like this. – Mgetz Oct 12 '17 at 16:02
  • Can you provide more code to illustrate your situation? Because the solution depends somewhat on what you are doing. The answer is you are not returning an array, you are returning a pointer. How to address that depends on your situation. – Galik Oct 12 '17 at 17:45

2 Answers2

3
Field fields[9];

Field *Table::getFields() {
    return fields;
}

The problem with your code is that you are returning a raw pointer (Field*), that has no clue of the size of the array it points to.

If you want to use range-based for loops in this case, you have to return something higher-level, for which a range is well defined.

For example, you can use std::array (which is a very tiny wrapper around raw C-style arrays), and return a reference to it (or const reference, for read-only access).

e.g.

// Instead of Field fields[9]
std::array<Field, 9> fields;

Another approach would be using std::vector<Field>, which has more overhead than std::array, but also offers many more features and is much more versatile. Again, you can define a vector<Field> data member, and return vector<Field> & from your getFields member function (or const vector<Field>& for read-only access).

Mr.C64
  • 37,988
  • 11
  • 76
  • 141
  • 1
    spans would also solve this issue, such as those provided by the [GSL](https://github.com/Microsoft/GSL) – Mgetz Oct 12 '17 at 15:59
  • 1
    “which is a very tiny wrapper around raw C-style arrays” To clarify, it’s very tiny in terms of C++ code. In probably all implementations, there is actually no run-time space overhead and for most operations no run-time size overhead. – Daniel H Oct 12 '17 at 16:02
1

The reason you can't use range based for loops is that you are not returning an array. Strictly speaking you are returning a pointer to the first element of the array. When passing or returning arrays in C++ they "decay" into a pointer to their first element.

To fix this you can return a reference to the array like this:

struct Table
{
    std::string stuff() const { return "stuff"; }
};

Table tables[9];

auto& getTables()
{
    return tables;
}

int main()
{
    for(auto table: tables)
        std::cout << table.stuff() << '\n';
}

I am using auto& here because the syntax for returning a reference to an array is somewhat convoluted:

Table (&getTables())[9] // using auto is nicer
{
    return tables;
}

Of course there are much nicer options available depending on your situation you might use std::vector<Table> or std::array<table> and return references to those.

Galik
  • 42,526
  • 3
  • 76
  • 100