2

I have this problem I'm trying to solve. Basically the base class has the function map, which takes a vector as input and outputs the final vector after some mapping function, in this case - f, has been performed. However, I'm really lost as to why when I print out 2*testVector - test1 in the main function, I get proper output, i.e. 6, -182 etc... but when I print out 2*testVector - test 2, it's still the same vector.

This happens both when I create "DoubleElements" twice or just call the same "DoubleElements" pointer twice (it only ever performs 1 map). Am I fundamentally missing some understanding? Any help is appreciated!

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

class RecursiveBase {
public: 
vector<int> map(vector<int> baseVector) {
    static int iter = 0;
     // Base case, return the final vector. 
    if (iter == 5) {
        return baseVector;
    // Replace the element with the old element mapped to the function.
    } else {
        baseVector[iter] = this->f(baseVector[iter]);
        iter++;
        return map(baseVector);
    }
}

private:
    virtual int f(int value) = 0;
};

class DoubleElements: public RecursiveBase {
private:
    int f(int value) {
        return 3*value;
    }
};

int main() {
    vector<int> testVector, o1, o2;
    testVector.push_back(3);
    testVector.push_back(-91);
    testVector.push_back(-42);
    testVector.push_back(-16);
    testVector.push_back(13);

    DoubleElements de;
    DoubleElements de1;

    RecursiveBase *test1 = &de;
    RecursiveBase *test2 = &de1;

    o1 = test1->map(testVector);
    o2 = test2->map(testVector);

    std::cout << "2*testVector - test1" << std::endl;
    for (unsigned int iter = 0; iter < o1.size(); iter++) {
        std::cout << o1[iter] << std::endl;
    }

    std::cout << "2*testVector - test2" << std::endl;
    for (unsigned int iter = 0; iter < o2.size(); iter++) {
        std::cout << o2[iter] << std::endl;
    }
}
Athena
  • 320
  • 2
  • 12
  • 1
    It's because your variable is static and thus shared among all instances of class `RecursiveBase`. So when you calculate map for `o1`, `iter` will be incremented to five. and when you calculate for `o2`, iter is already five and thus the same vector (`baseVector`) will be returned immediately – Mike van Dyke Apr 06 '18 at 08:33
  • 1
    You're missing an `override` for f() in your child class – babu646 Apr 06 '18 at 08:35

3 Answers3

2
static int iter = 0;

You should avoid declaring local static variables in methods unless 100% necessary.

The first call will increment iter to 5, but on the next call, iter, since it's static, will not reset it's value to 0.

As an example, a simple program like:

void test()
{
    static int x = 0;
    ++x;    
    cout << x << endl;
}

int main()
{        
    test();
    test();    
    return 0;
}

Will output

1
2
babu646
  • 989
  • 10
  • 22
1

From class.static.data/1:

A static data member is not part of the subobjects of a class.

For iter is static. It is part of the class RecursiveBase NOT part of the RecursiveBase objects.

To fix it, reset iter to 0:

if (iter == 5) {
   iter = 0; // reset iter
   return baseVector;
}

OUTPUT

2*testVector - test1
9
-273
-126
-48
39
2*testVector - test2
9
-273
-126
-48
39
Joseph D.
  • 11,804
  • 3
  • 34
  • 67
1

You can only ever call RecursiveBase::map once as it stands, because the iter is static. You also assume that you will only ever call it with a 5 element std::vector<int>, at which point std::array<int, 5> is a better choice.

If you want a recursive solution, instead pass the index as an additional parameter

public:
std::vector<int> map(std::vector<int> vec) {
    return do_map(vec, 0);
}
private:
std::vector<int> do_map(std::vector<int> & vec, std::size_t index) {
    if (index == vec.size()) { return vec; }
    vec[index] = f(vec[index]);
    return do_map(vec, ++index);
}

But that's still a gratuitous use of recursion. A much better solution is

public:
std::vector<int> map(std::vector<int> vec) {
    std::transform(vec.begin(), vec.end(), vec.begin(), [this](int i) { return f(i); });
    return vec;
}

You also have superfluous RecursiveBase * in your main

int main() {
    std::vector<int> testVector{3, -91, -42, -16, 13};

    DoubleElements de;
    DoubleElements de1;

  // declare at point of initialisation
  // don't need ->
    auto o1 = de.map(testVector);
    auto o2 = de1.map(testVector);

    std::cout << "2*testVector - test1" << std::endl;
    for (unsigned int iter = 0; iter < o1.size(); iter++) {
        std::cout << o1[iter] << std::endl;
    }

    std::cout << "2*testVector - test2" << std::endl;
    for (unsigned int iter = 0; iter < o2.size(); iter++) {
        std::cout << o2[iter] << std::endl;
    }

    return 0;
}
Caleth
  • 52,200
  • 2
  • 44
  • 75
  • Thanks for feedback, unfortunately since it is for an assignment, it has to be recursion, but the solution you offered is 10x more readable and logical. Cheers – Athena Apr 06 '18 at 09:38
  • The more I look at it, the more I think `RecursiveBase` is a class masquerading as `std::transform`. Does your instructor think all functions must belong to a class? – Caleth Apr 06 '18 at 09:47
  • Yes that's correct, mostly a tonne of smaller classes implementing different mapping functions like square rooting, tripling, another whole set of classes for filtering a vector. So for map it has to be like: MapBaseClass from which MapSquareRoot inherits etc. – Athena Apr 06 '18 at 10:29
  • uggh... `[this](int i) { return f(i); }` defines an instance of a class (which has no name), with a single method. In C++ you don't need a run-time polymorphic hierarchy *most* of the time, and when you do, there already exists `std::function<...>` – Caleth Apr 06 '18 at 10:32
  • I'm curious and want to learn more, do you know of any good resources on *proper* oop practices/programming practices in c++. I'm finding it a bit ridiculous that I have 22 .cpp and .h files combined to implement only 8 functions using the inheritance structure... – Athena Apr 06 '18 at 10:41
  • There's [the definitive book list](https://stackoverflow.com/q/388242/2610810). Note that C++ is multi-paradigm, OO isn't the be-all end-all, although if you take the C++ definition of "object" pedantically, everything is an object, and free-functions can be thought of as members of a "static namespace object" – Caleth Apr 06 '18 at 10:52