1

I am new too programming and am trying to pass an array of structures and an "option" to a function. I then want the function to be able to manipulate the data within the the array of the struct.

struct coordinates{
    int x;
    int y;
    int z;
}COORD;

The option parameter is an integer that will specify which part of the structure to manipulate.

  1. Manipulate x --- option = 0
  2. Manipulate y --- option = 1
  3. Manipulate z --- option = 2

An example of this is shown below. The function takes the array of structs and makes a 3 point moving average of the data-point specified data point. The issue I am having is that the program I am trying to build has 50+ members in each struct so writing out each if statement by hand is really tedious. What I am asking is if there is a more elegant solution???

#include <iostream>

struct coordinates{
    int x;
    int y;
    int z;
}COORD;

int main() {
    COORD points[10];

    // Initialising points
    for(int i = 0, i < 10, i++){ 
        points[i].x = 1;
        points[i].y = 2;
        points[i].z = 3;
    }
                            //(struct,startpoint,option)
    std::cout << movingaverage(&points,3,1); // Output
}

int movingaverage(COORD *data, int start_point, int option){

    if(option == 0) {
        // Assigns the "start" value of the sum value.
        sum = data[start_point]->x;
        sum = sum + data[start_point - 1]->x;
        sum = sum + data[start_point + 1]->x;
    }

    else if(option == 1) {
        // Assigns the "start" value of the sum value.
        sum = data[start_point]->y;
        sum = sum + data[start_point - 1]->y;
        sum = sum + data[start_point + 1]->y;
    }
    else if(option == 2) {
        // Assigns the "start" value of the sum value.
        sum = data[start_point]->z;
        sum = sum + data[start_point - 1]->z;
        sum = sum + data[start_point + 1]->z;
    }
    sum = sum / n;
    return sum; //Sum is the moving average
}
Wolf
  • 9,679
  • 7
  • 62
  • 108
silvergasp
  • 1,517
  • 12
  • 23
  • 3
    this will not compile. `COORD` is a variable, the type is `coordinates`. – Wolf May 30 '15 at 10:25
  • 3
    you can use and `std::vector` instead of a `struct`. Then your `option` is simply the index of the element. No need for `if`s – 463035818_is_not_an_ai May 30 '15 at 10:25
  • something with the comments for the options seem to be wrong. – Wolf May 30 '15 at 10:26
  • `points` would be a `std::vector>` – 463035818_is_not_an_ai May 30 '15 at 10:27
  • @tobi303 maybe as to generalize the processing of the options, but x, y, z seem to make more sense in other parts of the program – Wolf May 30 '15 at 10:29
  • @Wolf Then he should make a struct/class that encapsulates a vector with member functions to access the elements via their names x,y and z – 463035818_is_not_an_ai May 30 '15 at 10:31
  • 1
    @tobi303 It's also important if all members are of the same type. I think the `option` parameter is not the best solution. BTW: C++ provides also pointers to members. – Wolf May 30 '15 at 10:37
  • Are all members of the same type? If yes, would this be also true for the future? Templates may help for different member types, see [my answer](http://stackoverflow.com/a/30545301/2932052). – Wolf May 30 '15 at 11:00
  • The **title of this question is absolutely misleading**. For me, it's rather something like *generic member access* (If there would be a **"half downvote"**, I'd use it in this case). – Wolf May 30 '15 at 11:24
  • 1
    Nathan, there are lots of questionable answers. You need to clarify your question. Are the 50 fields the same type? Are their names meaningful? – Yakk - Adam Nevraumont May 30 '15 at 12:01
  • @Wolf I apologise if the title was misleading if you have any suggestions I would be happy to change the title to avoid future confusion. – silvergasp May 30 '15 at 12:16

4 Answers4

1

C++ does not support reflection and therefore you cannot iterate over members of a struct. The way I would do this is with a getter function.

template<class Getter>
int movingaverage(COORD *data, int start_point, const Getter &get){
    auto sum = get(data[start_point]);
    sum = sum + get(data[start_point - 1]);
    sum = sum + get(data[start_point + 1]);
    return sum;
}

std::cout << "x: " << movingaverage(&points, 3, [](const COORD &coord){return coord.x;}) << '\n';
std::cout << "y: " << movingaverage(&points, 3, [](const COORD &coord){return coord.y;}) << '\n';
std::cout << "z: " << movingaverage(&points, 3, [](const COORD &coord){return coord.z;}) << '\n';

If you or your compiler are confused about the lambda part you can just write a regular function and pass that instead.

nwp
  • 9,623
  • 5
  • 38
  • 68
  • It does the trick, but with all due respect, I would hate to read such code (especially when template and call are in separate files), and code is read much more times than it is written. However, replacing lambdas with named functions such as OPTION0 (which functions are placed right near the template declaration), the whole thing will become quite readable. – No-Bugs Hare May 30 '15 at 11:02
  • Not bad, but if the compiler does not understand lambdas, [members to pointers](http://stackoverflow.com/a/30545301/2932052) are an option. – Wolf May 30 '15 at 11:10
1

Also pointer to members are a possible solution; in combination with a template function, the members can be of different type. But, yes, the syntax is something strange:

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

/// calculate the sum of a given structure member in a vector:
template <typename T, typename M>
M membersum(vector<T> array, M (T::*mptr)) {
    M sum = 0;
    for (int i=0; i<array.size(); i++) {
        sum += (array[i].*mptr);
    }
    return sum;
}

struct Point {
    Point(int x, int y, float z): x(x), y(y), z(z) {}
    int x;
    int y;
    float z;
};

int main() {
    vector<Point> points;
    points.push_back(Point(1,2,3.8));
    points.push_back(Point(1,2,4.5));
    points.push_back(Point(1,2,1.7));
    // your code goes here
    cout << "sum x: " << membersum(points, &Point::x) << endl;
    cout << "sum y: " << membersum(points, &Point::y) << endl;
    cout << "sum z: " << membersum(points, &Point::z) << endl;
    return 0;
}

This example I created on ideone.com.

Wolf
  • 9,679
  • 7
  • 62
  • 108
  • Better than mine (the one with simple encapsulation), and better than lambdas (which are not readable, and making them readable will be bulkier than this one). – No-Bugs Hare May 30 '15 at 11:13
  • @No-BugsHare Thanks. The member pointers are very readable on the caller side `&Point::x`, but the syntax you see in the implementation of `membersum` is very special to people that never have seen it before. – Wolf May 30 '15 at 11:20
  • Yes, but I'd rather have not-so-easy-to-read stuff well-encapsulated within membersum(), than spread all over the code as in lambda-based solution. – No-Bugs Hare May 30 '15 at 11:27
  • @No-BugsHare True. What I like best is the template part that isolates the algorithm from the type of the current member. – Wolf May 30 '15 at 11:32
  • @Wolf thanks so much for your answer, I've been stuck on this for quite a while now. – silvergasp May 30 '15 at 12:15
0

You can try something along the lines of

class coordinates{
    private:
    int xyz[3]; //implementation detail(!)

public:
    int x() const { return xyz[0]; }
    int y() const { return xyz[1]; }
    int z() const { return xyz[2]; }

    //some relevant setters go here

    static int movingaverage(COORD *data, int start_point, int option){

        sum = data[start_point]->xyz[option];
        sum = sum + data[start_point - 1]->xyz[option];
        sum = sum + data[start_point + 1]->xyz[option];
    }
};
Wolf
  • 9,679
  • 7
  • 62
  • 108
No-Bugs Hare
  • 1,557
  • 14
  • 15
  • 1
    The idea is good, but `movingaverage` as a member function should not take `data` as input but work on its private data. Anyhow the function cannot access `xyz` of `data` as you declared it as private. – 463035818_is_not_an_ai May 30 '15 at 10:42
  • @tobi303: huh? movingaverage is a member function, so it can access private members, and data is a function parameter without restrictions. – No-Bugs Hare May 30 '15 at 10:55
  • the formatting could be better ;-) ...i tried to reindent it. – Wolf May 30 '15 at 11:43
  • I'd try to omit the encapsulation part, because the question used a struct. The idea of changing a lot of members to an array is the most important one, right? – Wolf May 30 '15 at 11:47
  • @Wolf: well, the difference between class and struct is nominal, but I think the solution here should be two-fold: one half of solution is array (as was pretty much suggested by tobi303 in comments - though he suggested a vector), and another half is encapsulating this array so it cannot be abused (and implementation can be changed later). Still, your pointer-to-member solution is IMHO clearly better. – No-Bugs Hare May 30 '15 at 12:45
0

I would suggest to use a simple functor.

Class MovingAvg
{
MovingAvg(std::vector<Coordinate> *pSet);
void SetPointSet(std::vector<Coordinate> *p);

double operator()(int startPoint, int option)
{
   //calculation here. 
   double sum = 0;
   for(int i = -1; i <= 1; ++i)
   {
       switch(option)
         case 0:
           sum += pointSet[startPoint + i].x; //check for bounday conditions.
    .................

   }
   return sum / 3;
}

private:
//container
std::vector<Coordinate> *pointSet;
}

Use it as.

MovAvg mvG(pointSet);
double sum = mvG(startP, option); 

//change the private member if the point set must be changed.

Functors are pretty handy in cases like this.

  • This doesn't address the lengthy switch-case construction. I don't see why *`Functors are pretty handy in cases like this.`* – Wolf May 30 '15 at 11:33
  • For all the reasons listed here, http://stackoverflow.com/questions/6451866/why-use-functors-over-functions The problem is well described with functor and easy to extend and read. It is not always about less code in MHO. You can still make this better without a switch by templating as in example above. – code_not_yet_complete May 30 '15 at 11:45