vector of pointers to a vector of objects, pointer or iterator?

[LMHmedchem]

Hello,

I have allot of data stored in a vector of the following objects,

class row_data {

public:

   // class constructor
   row_data()
      : point_B(false),
        set_A(false),
        set_B(false),
        inconclusive(false),
        extreme_value(false),
        index(0),
        id(0),
        col_min(-1),
        col_max(-1),
        col_min_value(0.0),
        col_max_value(0.0),
        c_distance(0.0),
        row_name("") { }

   // member variables
   bool point_B, set_A, set_B, inconclusive, extreme_value;
   unsigned int index, id;
   int col_min, col_max;
   double col_min_value, col_max_value;
   double c_distance;
   std::string row_name;
   std::vector<std::string> input_string_data;
   std::vector<int> int_data;
   std::vector<double> double_data;

};

There can be allot of data in the vector<row_data> container and there are times I need to sort on a member variable or process part of the data. I think that the easiest thing to do would be to create a vector of pointers to the specific data objects I need and then sort the vector of pointers, pass the vector of pointers to various functions etc.

I was thinking of something like,

// function object to locate object in vector of object by row_name
class match_name {
   public:
   explicit match_name(const string& row_name) : search_string(row_name) {}

   bool operator()(const row_data& current_row) const { return current_row.row_name == search_string; }
   private:
      string search_string;
};


// find an object in a vector of object and assign location to a pointer, return the pointer
row_data* row_object_by_row_name(const string& row_name, const vector<row_data>& data_rows) {

   // declare iterator to location in vector of row_data objects
   vector<row_data>::iterator row_it;

   // find object location by row name member
   row_it = find_if( data_rows.begin(), data_rows.end(), match_name(row_name) );

   // assign address of iterator to pointer
   row_data* found_row = &(row_it*);

// return object
return found_row;
}

// main data container
vector<row_data> data_rows;

// lookup value
string look_for_row;

// assign value
look_for_row.assign("some_row_name");

// container
vector<row_data*> data_subset;

// assign address of iterator to pointer
row_data* found_row = row_object_by_row_name(look_for_row, data_rows)

// find object in vector and add pointer to vector of pointer
data_subset.push_back(found_row);

// do things with data_subset

The final block of code would be in its own function that would return the populated vector of pointers.

Is this a reasonable approach?Is there any reason to not use a vector of iterators instead of converting the iterator address to a pointer? How would I go about sorting the vector of pointers on a member variable in data_rows?

Also, I think that the class to find the object by row_name should just be a member function in the row_data class but I'm not quite sure what that would look like.

Thanks,

LMHmedchem

[wolle]

I have allot of data

What's a lot?

Firstly, sequential searches will take you a long way. It's simple and straightforward and often fast enough.

Secondly, what you are considering is to build your own small database. An alternative would be to use a third party free database. There are many options both SQL and non-SQL for large and small needs, in-memory up to cloud based..

If it's more than just a toy program you're writing I think you should seriously consider the above two options before you decide to roll your own. If it's an exercise I think you should go ahead with your ideas and see where they take you. That's the best way to learn.

Finally I suggest you take this opportunity to familiarize yourself with the basics of computer science such as data abstraction, information hiding, encapsulation, standard data structures, memory management options, the OO and functional paradigms, design patterns, etcetera. It will allow you to better utilize the power of C++.

Good luck!

[2kaud]

Hmmm.

// assign address of iterator to pointer
   row_data* found_row = &(row_it*);

This has a fundamental flaw in that you are not checking that the result of find_if() actually found it - and are potentially trying to obtain the address of the contents of memory which may not be valid! Why have the function row_object_by_row_name() at all - it doesn't really add anything.

I would have a vector of iterators rather than a vector of pointers - an iterator can be dereferenced just like a pointer. However, you need to be aware that whether you hold a vector of iterators or a vector of pointers, once you populate data_subset you can't change data_rows. Simply, if you add or remove any element(s) to data-rows you invalidate all the stored pointers/iterators in data_subset and will have to re-populate this vector in its entirety. (advanced - not quite but unless you know absolutely what's going on, this is how you treat it). Consider

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

class row_data {

public:

	// class constructor
	row_data()
		: point_B(false),
		set_A(false),
		set_B(false),
		inconclusive(false),
		extreme_value(false),
		index(0),
		id(0),
		col_min(-1),
		col_max(-1),
		col_min_value(0.0),
		col_max_value(0.0),
		c_distance(0.0),
		row_name("") { }

	// member variables
	bool point_B, set_A, set_B, inconclusive, extreme_value;
	unsigned int index, id;
	int col_min, col_max;
	double col_min_value, col_max_value;
	double c_distance;
	std::string row_name;
	std::vector<std::string> input_string_data;
	std::vector<int> int_data;
	std::vector<double> double_data;

};

// function object to locate object in vector of object by row_name
class match_name {
public:
	explicit match_name(const string& row_name) : search_string(row_name) {}

	bool operator()(const row_data& current_row) const { return current_row.row_name == search_string; }
private:
	string search_string;
};

using veccit = vector<row_data>::const_iterator;

int main()
{
	// main data container
	vector<row_data> data_rows;

	// lookup value
	string look_for_row;

	// assign value
	look_for_row.assign("some_row_name");

	// container
	vector<veccit> data_subset;
	veccit found_row;

	if ((found_row = find_if(data_rows.begin(), data_rows.end(), match_name(look_for_row))) != data_rows.end())
		data_subset.push_back(found_row);

	// do things with data_subset
}

Also be aware that for every look-up value, you are potentially searching the whole of data_rows. This is very inefficient - especially if data_rows contain a lot(??) of data. If you have a subset vector then obviously you are expecting to look up at least several values. It would be better to do this all at once with a single pass of data_rows. So I would do it so that you first built a vector of look-up values and from this, build the data_subset vector.

Something like (for c++98 - its easier in c++17!) [not tried]

using veccit = vector<row_data>::const_iterator;

class match_name {
public:
	explicit match_name(veccit current_row) : row(current_row) {}

	bool operator()(const string& search_name) const { return row->row_name == search_name; }
private:
	veccit row;
};

int main()
{
	vector<row_data> data_rows;
	vector<string> find_names;
	vector<veccit> data_subset;

	find_names.push_back("some_row_name");

	for (veccit datait = data_rows.begin(); datait != data_rows.end(); ++datait) {
		vector<string>::const_iterator stit = find_if(find_names.begin(), find_names.end(), match_name(datait));

		if (stit != find_names.end())
			data_subset.push_back(datait);
	}

	// do things with data_subset
}

[2kaud]

if you are going to crate many data_subsets from data_rows, it will probably be better for performance to crate an intermediate map of row_name to iterator and then use this map as the basis of building data_subsets. Something like

using Veccit = vector<row_data>::const_iterator;
using Mapcit = map<string, Veccit>::const_iterator;
using Vstrit = vector<string>::const_iterator;

class match_name {
public:
	explicit match_name(Veccit current_row) : row(current_row) {}

	bool operator()(const string& search_name) const { return row->row_name == search_name; }
private:
	Veccit row;
};

int main()
{
	vector<row_data> data_rows;
	vector<string> find_names;
	vector<Veccit> data_subset;
	map<string, Veccit> mvec;

	// Populate data_rows here

	// Populate the map for name to iterator
	// Note. Don't change data_rows now!
	for (Veccit vit = data_rows.cbegin(); vit != data_rows.cend(); ++vit)
		mvec[vit->row_name] = vit;

	// Populate names to find
	find_names.push_back("some_row_name");

	// Populate data_subset
	for (Vstrit vsit = find_names.begin(); vsit != find_names.end(); ++vsit) {
		Mapcit mfnd = mvec.find(*vsit);

		if (mfnd != mvec.end())
			data_subset.push_back(mfnd->second);
	}

	// do things with data_subset
}

[2kaud]

If you want to sort on a particular element of row_data, then you use sort() with the appropriate compare function. Eg. if you wish to sort on c_distance, then consider

bool sort_dist(Veccit a, Veccit b)
{
	return a->c_distance < b->c_distance;
}
....

// after data_subset has been populated
sort(data_subset.begin(), data_subset.end(), sort_dist);

Another way would be keep a (multi)set for data_subset rather than a vector. You could still iterate through the elements - but not access them individually via []. Consider (not tried)

#include <vector>
#include <string>
#include <algorithm>
#include <map>
#include <set>
#include <iostream>
using namespace std;

class row_data {

public:
	// class constructor
	row_data()
		: point_B(false),
		set_A(false),
		set_B(false),
		inconclusive(false),
		extreme_value(false),
		index(0),
		id(0),
		col_min(-1),
		col_max(-1),
		col_min_value(0.0),
		col_max_value(0.0),
		c_distance(0.0),
		row_name("") { }

	// member variables
	bool point_B, set_A, set_B, inconclusive, extreme_value;
	unsigned int index, id;
	int col_min, col_max;
	double col_min_value, col_max_value;
	double c_distance;
	std::string row_name;
	std::vector<std::string> input_string_data;
	std::vector<int> int_data;
	std::vector<double> double_data;
};

using Veccit = vector<row_data>::const_iterator;
using Mapcit = map<string, Veccit>::const_iterator;
using Vstrit = vector<string>::const_iterator;

// Set insertion comparison
struct setcomp
{
	bool operator()(Veccit a, Veccit b)
	{
		return a->c_distance < b->c_distance;
	}
};

using Setcit = multiset<Veccit, setcomp>::const_iterator;

int main()
{
	vector<row_data> data_rows;
	vector<string> find_names;
	multiset<Veccit, setcomp> data_subset;
	map<string, Veccit> mvec;

	// Populate data_rows here

	// Populate the map for name to iterator
	// Note. Don't change data_rows now!
	for (Veccit vit = data_rows.cbegin(); vit != data_rows.cend(); ++vit)
		mvec[vit->row_name] = vit;

	// Populate names to find
	find_names.push_back("some_row_name");

	// Populate data_subset
	// This will populate multiset in c_distance order
	for (Vstrit vsit = find_names.begin(); vsit != find_names.end(); ++vsit) {
		Mapcit mfnd = mvec.find(*vsit);

		if (mfnd != mvec.end())
			data_subset.insert(mfnd->second);
	}

	// Do things with data_subset
	for (Setcit ds = data_subset.begin(); ds != data_subset.end(); ds = next(ds))
		cout << (*ds)->row_name << " " << (*ds)->c_distance << endl;
}

[LMHmedchem]

Sorry for the delay, I spent most of yesterday with the computer off because of allot of local lightning.

What's a lot?

In this case, there are about 2000 rows of data. For each row, there is a vector<string> with 50 elements (which I could clear), a vector<double> with 50 elements, and the other assorted member variables in the class. That is not a great deal in computing terms but is starting to be big enough to get cumbersome when sorting and making copies. This could be used on data sets with 10^5 to 10^7 rows if I ever get the algorithm working. (which is a sizable if)

Firstly, sequential searches will take you a long way. It's simple and straightforward and often fast enough.

Well that's good because I do an awful lot of them. I learned programming when the about the only control element was a DO loop, CONTINUE, or GOTO and it's hard to get out of that kind of thinking. I don't think I have ever written a program that didn't have at least one double loop somewhere.

Secondly, what you are considering is to build your own small database. An alternative would be to use a third party free database. There are many options both SQL and non-SQL for large and small needs, in-memory up to cloud based..

Yes, if I was going to leverage 10^6 rows or more I would look at connecting to a database. I have done that with ruby and sqlite3 but I don't know it very well.

If it's more than just a toy program you're writing I think you should seriously consider the above two options before you decide to roll your own. If it's an exercise I think you should go ahead with your ideas and see where they take you. That's the best way to learn.

The programs I write are for my research. They are tools to do data processing that I can't (or wouldn't want to) do by hand and need to be able to document, repeat, and adjust. Because it's research, I usually don't entirely/completely know what I want the processing algorithm to do, let alone how it can best be implemented. Much of the purpose of the program is to give me the output I need to figure out how I want the program to work, if that makes any sense. This makes the programming a moving target. If the algorithm I come up with is useful, I tend to keep working on the program to make it more stable, efficient, to add features, etc. I have many programs I only took so far because the algorithm never developed into anything workable, or useful even if it worked. As Einstein said, "If we knew what it was we were doing, it would not be called research, would it?".

Finally I suggest you take this opportunity to familiarize yourself with the basics of computer science such as data abstraction, information hiding, encapsulation, standard data structures, memory management options, the OO and functional paradigms, design patterns, etcetera. It will allow you to better utilize the power of C++.

I am always trying to strike the balance between developing the tools I need for my research and learning how to develop those tools. I know something about most of what you list above but almost never enough to apply it without assistance. I do very much appreciate the input I get here and at other programming forums. I would not make much progress without it.

LMHmedchem

[LMHmedchem]

// assign address of iterator to pointer
   row_data* found_row = &(row_it*);

This has a fundamental flaw in that you are not checking that the result of find_if() actually found it - and are potentially trying to obtain the address of the contents of memory which may not be valid! Why have the function row_object_by_row_name() at all - it doesn't really add anything.

Generally I would have already confirmed that an object was created corresponding to each name, but I guess there are other reasons why find_if() could fail. I need to add allot more exception handling to everything I write.

I would have a vector of iterators rather than a vector of pointers - an iterator can be dereferenced just like a pointer. However, you need to be aware that whether you hold a vector of iterators or a vector of pointers, once you populate data_subset you can't change data_rows. Simply, if you add or remove any element(s) to data-rows you invalidate all the stored pointers/iterators in data_subset and will have to re-populate this vector in its entirety. (advanced - not quite but unless you know absolutely what's going on, this is how you treat it).

There is no important reason why I would need to change the main data container once it is populated on data read in. I do change values in the populated objects, such as the value of one of the bools or doubles. This is to store the results of processing with the original data. I would never be adding, removing, or moving, objects in the container once they have been created.

As long as object member variables were initialized to a value, it seems like I should be able to change the value of such variables without invalidating the address of existing iterators. The new value would not take up any more memory then the initial value so I wouldn't expect that to change the addressing. Is that true, or can I not change anything once iterators have been generated to point to specific objects? It seems like adding to a vector in an object could be a problem because you are dynamically changing the allocated memory.

If I can't change anything and have to treat the main data container as "read only" I can always store processing results somewhere else, such as by accumulating my result objects when they are returned.

using veccit = vector<row_data>::const_iterator;

What is the purpose of the "using" declaration here? I more or less understand that you can add part of a namespace with this, such as "using std::vector", instead of including the entire namespace, or you can include data members declared in one class in another class with this. Is there some reason you would not just declare the iterator in main() or use a typedef in the header file?

if ((found_row = find_if(data_rows.begin(), data_rows.end(), match_name(look_for_row))) != data_rows.end())
        data_subset.push_back(found_row);

You have eliminated one function that isn't doing anything and added the exception handling for the case where the name to match is not found. I would want to exit in the case where that happens, so I guess it would look like,

if( (found_row = find_if(data_rows.begin(), data_rows.end(), match_name(look_for_row))) != data_rows.end() ) {
   data_subset.push_back(found_row);
}
else {
   cerr < "name " << look_for_row << " was not found in data container" << endl;
   exit(-1);
}

Also be aware that for every look-up value, you are potentially searching the whole of data_rows. This is very inefficient - especially if data_rows contain a lot(??) of data. If you have a subset vector then obviously you are expecting to look up at least several values. It would be better to do this all at once with a single pass of data_rows. So I would do it so that you first built a vector of look-up values and from this, build the data_subset vector.

for(veccit datait = data_rows.begin(); datait != data_rows.end(); ++datait) {
   vector<string>::const_iterator stit = find_if(find_names.begin(), find_names.end(), match_name(datait));
   if(stit != find_names.end())
         data_subset.push_back(datait);
}

So you iterate through data_rows and for each object, check the object row_name value against all of the names in the find_names vector because it is less expensive to loop through the name list n times than to loop on rows_data n times? If this was a real performance issue I could always store the row objects in a map indexed on the name value instead of a vector.

If you want to sort on a particular element of row_data, then you use sort() with the appropriate compare function. Eg. if you wish to sort on c_distance, then consider

bool sort_dist(Veccit a, Veccit b)
{
    return a->c_distance < b->c_distance;
}

// after data_subset has been populated
sort(data_subset.begin(), data_subset.end(), sort_dist);

I have done this kind of thing before to sort a vector of objects but I didn't know how different it would be to sort a vector of iterators pointing to objects.

One thing I know I will want to do is something like find all of the objects in the data_rows where one of the bools has a given value. Then I would want to sort the accumulated vector of iterators on some other value.

I guess it would look something like,

#include <vector>
#include <string>
#include <algorithm>

using namespace std;

// iterator type
using veccit = vector<row_data>::const_iterator;

// custom operator to sort vector of iterators on object member c_distance, small to large
bool sort_sm2lg_c_distance(veccit a, veccit b) { return a->c_distance < b->c_distance; }

// subset container
vector<veccit> data_subset;

// loop through data container and look for objects where bool set_B=+true
for(unsigned int i=0; i<data_rows.size(); i++) {

   // test if set_B==true for object i
   if(data_rows[i].set_B) {

      // declare iterator and initialize location
      veccit vec_location == row_data.begin();

      // set iterator to position of object i
      // should this advance by i or i-1???
      advance(vec_location, i);

      // add iterator to vector
      // iterator goes out of scope at end of if so no need to reinitalize
      data_subset.push_back(vec_location);

   }
}

// after data_subset has been populated
sort(data_subset.begin(), data_subset.end(), sort_sm2lg_c_distance);

To me, the method of searching data_rows and assigning the iterator location looks a bit iffy. Is there some way to do this more similar to find_if, as opposed to looping through the container? I suppose I could make my own find_all() function, but it would just look like the above I think.

Another way would be keep a (multi)set for data_subset rather than a vector. You could still iterate through the elements - but not access them individually via [].

I will look at this, I have never used a multiset before.

LMHmedchem

[2kaud]

I do change values in the populated objects, such as the value of one of the bools or doubles.

Changing the values of data_rows after the container of iterators has been produced is fine as the underlying allocated vector memory won't be re-allocated.

It seems like adding to a vector in an object could be a problem because you are dynamically changing the allocated memory.

Yes - and erase as well.

what is the purpose of the "using" declaration here?...or use a typedef in the header file

using in this context is the same as typedef - and typedef could be used here.

[2kaud]

One thing I know I will want to do is something like find all of the objects in the data_rows where one of the bools has a given value. Then I would want to sort the accumulated vector of iterators on some other value.

Why not simply (not tried)

using Veccit = vector<row_data>::const_iterator;
using Subcit = vector<Veccit>::const_iterator;

// custom operator to sort vector of iterators on object member c_distance, small to large
bool sort_sm2lg_c_distance(Veccit a, Veccit b) { return a->c_distance < b->c_distance; }

int main()
{
	vector<row_data> data_rows;
	vector<Veccit> data_subset;

	// Populate data_rows here

	// loop through data container and look for objects where bool set_B=+true
	for (Veccit dit = data_rows.begin(); dit != data_rows.end(); ++dit)
		// test if set_B==true
		if (dit->set_B)
			data_subset.push_back(dit);

	// after data_subset has been populated
	sort(data_subset.begin(), data_subset.end(), sort_sm2lg_c_distance);

	// Do things with data_subset
	for (Subcit ds = data_subset.begin(); ds != data_subset.end(); ds = next(ds))
		cout << (*ds)->row_name << " " << (*ds)->c_distance << endl;
}

[2kaud]

If you only work with one data subset at a time, then another way to consider would be to partition data_rows so that all the required values come before all the non-required values. That way you don't need to build new vectors - but you can only deal with one subset at a time as for each subset, data_rows will have to be re-partitioned. You only then sort the part of data_rows that contains the required data. Consider (not tried)

using Vecit = vector<row_data>::iterator;

bool sort_sm2lg_c_distance(const row_data& a, const row_data& b) { return a.c_distance < b.c_distance; }

bool bysetB(const row_data& rd) { return rd.set_B;}

int main()
{
	vector<row_data> data_rows;

	// Populate data_rows here

	Vecit partit = partition(data_rows.begin(), data_rows.end(), bysetB);

	sort(data_rows.begin(), partit, sort_sm2lg_c_distance);

	// Do things with data_rows
	for (Vecit dr = data_rows.begin(); dr != partit; ++dr)
		cout << dr->row_name << " " << dr->c_distance << endl;
}

PS Whilst you are doing your research and figuring things out with a small data set, this partition/sort approach may be the easiest as you can write different extraction/sort 1 line functions to produce fairly quickly the required data. Once you know 'what you want' and are ready to do it with the much, much larger data set then that is the time to start looking at performance etc.

[LMHmedchem]

I am doing some of the basic work to collect a subset and loop on it.

I am getting a compiler error from the function that is collecting the subset,

// row_data class defined
class row_data { }

// iterator to specific object in a vector of row_data
// not using const_iterator in case we are changing data values in row_data
typedef std::vector<row_data>::iterator row_data_vec_it;

// main data container created and populated
vector<row_data> data_rows;

// function called to create data subset

// create data subset, returns a vector of iterators to specific objects in a vector of row_data objects
vector<row_data_vec_it>  get_data_subset(const vector<row_data>& data_rows) {

   // subset container to return
   vector<row_data_vec_it> data_subset;

   // declare iterator an set to first element in vector
   row_data_vec_it  location;

   // loop through data container and look for objects where bool set_B=+true
   for(location = data_rows.begin(); location != data_rows.end(); ++location) {
      // test if set_B==true and add to vector
      if(location->set_B) { data_subset.push_back(location); }
   }

return data_subset;
}

I am getting the following compiler error,

error: no match for ‘operator=’ (operand types are ‘row_data_vec_it {aka__gnu_cxx::__normal_iterator<row_data*, std::vector<row_data> >}’ and ‘std::vector<row_data>::const_iterator {aka __gnu_cxx::__normal_iterator<const row_data*, std::vector<row_data> >}’)
for(location = rows_data.begin(); location != rows_data.end(); ++location) {

It is the location = data_rows.begin() statement that it doesn't like. If I use a const_iterator in the typedef it compiles fine but I cannot use the iterator to change values in data_rows. I also tried location = rows_data.front() instead of begin() since I though I might need to assign the reference returned by front() but I get the same error.

I have seen countless references to code like,

std::vector<int>::iterator it = myvector.begin()

so I don't see what I am doing wrong here.

The iterator typedef is currently in the header file with the class definition since I thought that was a good place to keep it. I had trouble compiling with using so I stayed with the typedef.

LMHmedchem

[LMHmedchem]

I thought it would be something stupid, I can't pass my data container to the function as const and then assign the location .begin() to a normal iterator. Unless there is some other method or iterator type, I have to choose between passing the data container as const and being able to use the iterator to change a value in the data container after processing.

There are other ways to store my results so I guess I will create another structure to accumulate processing results unless something else is recommended.

LMHmedchem

[LMHmedchem]

Changing to a vector of iterators has created a quandary with some of my functions. When I was looping on my vector of data objects, I would pass object member variables to functions directly by reference.

// class for data container
class row_data {

public:

   // class constructor
   row_data() : set_A(false), set_B(false), id(0), c_distance(0.0), row_name("")  { }

   bool set_A, set_B;
   unsigned int id;
   double c_distance;
   std::string row_name;
   std::vector<double> double_data;

};

// find the vector between two points using coordinates passed to function
// point A is the source point and point B is the destination point
vector<double> vector_between_points(vector<double>& src, vector<double>& dest) {

   // return container, vector between A and B
   vector<double> vec_A_to_B;

   // for each element in the destination point, subtract the corresponding element
   // in source point and add to result as an element in vector BA
   for(unsigned int i=0; i<src.size(); i++ ) { vec_A_to_B.push_back(dest[i] - src[i]); }

return vec_A_to_B;
}

// data container
vector<row_data> data_rows;

// create vector between first two data points
vector<double> vAB = vector_between_points(data_rows[i].double_data, data_rows[j].double_data);

Now that I am working with a vector of iterators I need something more like,

// type for vector of row data iterator
typedef std::vector<row_data>::const_iterator row_data_vec_it;

// two specific rows
row_data_vec_it row_A = data_rows.begin();
row_data_vec_it row_B = data_rows.begin()+1;

// create vector between first two data points
vector<double> vAB = vector_between_points( (*row_A).double_data, (*row_B).double_data );

Setting aside how I referenced the row_A and row_B iterators (the above is the simplest thing I could think of), I am passing the wrong type to the function which is expecting a reference to a vector of double. I thought that maybe I could do,

// create vector between first two data points
vector<double> vAB = vector_between_points( &(*row_A).double_data, &(*row_B).double_data );

to pass the address where the iterator is pointing, but that doesn't compile either.

Is there some syntax to accomplish what I want here or do I need to rewrite my functions to expect an iterator to be passed in the call? I could do something like make a local copy of the variable, but that defeats the purpose of using the vector of iterators in the first place.

LMHmedchem

[2kaud]

Setting aside how I referenced the row_A and row_B iterators (the above is the simplest thing I could think of), I am passing the wrong type to the function which is expecting a reference to a vector of double. I thought that maybe I could do,

I don't know how vector_between_points() is defined, but the below compiles with VS2017.

void vector_between_points(const vector<double>& dd1, const vector<double>& dd2) { }

int main()
{
	typedef std::vector<row_data>::const_iterator row_data_vec_it;

	vector<row_data> data_rows;

	row_data_vec_it row_A = data_rows.begin();
	row_data_vec_it row_B = data_rows.begin() + 1;

	// create vector between first two data points
	vector_between_points(row_A->double_data, row_B->double_data);
}

[LMHmedchem]

I don't know how vector_between_points() is defined, but the below compiles with VS2017.

The function definition for vector_between_points() is in the first code block in post 13, but I wasn't passing the vector of double to vector_between_points() as a constant reference. Changing that in the function definition fixes the error.

I have had some of these functions for a while and they aren't as clean as they should be. My assumption is that every container should be passed a const & unless the function needs its own copy or needs to make changes in the original so I will check through and make sure that has been done consistently.

It's interesting that you have declared a second kind if iterator to iterate over the vector of iterators.

To loop over the sorted subset and create every triplet of rows I would have done something like.

// includes
#include <vector>
#include <string>
#include <iterator> // for object sort
#include <algorithm> // for object sort

// add namespace
using namespace std;

// class for data container
class row_data {

public:

   // class constructor
   row_data() : set_A(false), set_B(false), id(0), c_distance(0.0), row_name("")  { }

   // member variables
   bool set_A, set_B;
   unsigned int id;
   double c_distance;
   std::string row_name;
   std::vector<double> double_data;

};

// const iterator to specific object in a vector of row_data
// cannot be used to change member values in a vector of row data, just read
typedef std::vector<row_data>::const_iterator row_data_vec_it;

// custom operator to sort vector of iterators on object member c_distance, small to large
bool sort_vec_it_sm2lg_c_distance(row_data_vec_it a, row_data_vec_it b) { return a->c_distance < b->c_distance; }

// main data container
vector<row_data> data_rows;

// populate data_rows

// data subset container
vector<row_data_vec_it> data_subset;

// loop through data container and look for objects where bool set_B==true
// would be a function to return data_subset
for(row_data_vec_it location = data_rows.begin(); location != data_rows.end(); ++location) {
   // test if set_B==true
   if(location->set_B) { data_subset.push_back(location); }
}

// after data_subset has been populated, sort on c_distance
sort(data_subset.begin(), data_subset.end(), sort_vec_it_sm2lg_c_distance);

// now we have a sorted subset of the original data

// loop over sorted subset and create all sets of three points
// loop to size()-2 because the last 2 elements are redundant for i
for(unsigned int i=0; i<data_subset.size()-2; i++) {

   // iterator to point_B
   row_data_vec_it point_B = data_subset.begin + i;

   // loop on i+1 to skip current B
   // loop to size()-1 because the last elements is redundant for j
   for(unsigned int j=i+1; j<data_subset.size()-1; j++) {

      // iterator to point_A
      row_data_vec_it point_A = data_subset.begin + j;

      // loop on j+1 to skip current A
      for(unsigned int k=j+1; k<data_subset.size(); k++) {

         // iterator to point_D
         row_data_vec_it point_D = data_subset.begin + k;

         // print some stuff
         cout "current set of three points is" << endl;
         cout "point_B: " << point_B->row_name << endl;
         cout "point_A: " << point_A->row_name << endl;
         cout "point_D: " << point_D->row_name << endl;

         // pass to a function
         do_things_with_three_points(point_B, point_A, point_D);

         // etc

      }
   }
}

This is redundant in that I an declaring new iterators instead of using the iterators that are already stored in the vector. Am I right that you can't just use (*data_subset[i]) as if it were an iterator?

You have declared a new type of iterator for the vector of iterators

// use to loop through a vector of row_data_vec_it
typedef std::vector<row_data_vec_it>::const_iterator vec_subset_it;

// iterate over subset vector
for(vec_subset_it subset_row = data_subset.begin(); subset_row != data_subset.end(); ++subset_row) {
  cout << (*subset_row)->row_name << " " << (*subset_row)->c_distance << endl;
}

It looks like this needs something like a double de-reference to access the original object values, (*subset_row)->c_distance. Is that correct?

I'm no sure how I would set this up like my example above to create all sets of three.

Maybe something like,

// use to loop through a vector of row_data_vec_it
typedef std::vector<row_data_vec_it>::const_iterator vec_subset_it;

// iterate over sorted subset and create all sets of three points
for(vec_subset_it  row_B = data_subset.begin(); row_B != data_subset.end() -2; ++row_B) {

   for(vec_subset_it  row_A = advance(row_B,1); row_A != data_subset.end() -1; ++row_A) {

      for(vec_subset_it  row_D = advance(row_A,1); row_D != data_subset.end(); ++row_D) {

         // print some stuff
         cout "current set of three points is" << endl;
         cout "point_B: " << (*row_B)->row_name << " " << (*row_B)->c_distance << endl;
         cout "point_A: " << (*row_A)->row_name << " " << (*row_A)->c_distance << endl;
         cout "point_D: " << (*row_D)->row_name << " " << (*row_D)->c_distance << endl;

      }
   }
}

LMHmedchem