No longer crashing on rowset/find

git-svn-id: svn+ssh://gitlab/srv/svn-repos/pdb-redo/trunk@523 a1961a4f-ab94-4bcc-80e8-33b5a54de466

include/cif++/Cif++.h

@@ -97,6 +97,7 @@ class File;
 class Datablock;
 class Category;
 class Row;			// a flyweight class that references data in categories
+class RowSet;
 class Item;
 class Validator;
 
@@ -430,30 +431,20 @@ class Row
 	friend class CatIndex;
 	friend class RowComparator;
 	friend class detail::ItemReference;
+	friend class RowSet;
 
-	Row(ItemRow* data = nullptr, bool cascadeUpdate = true)
-		: mData(data), mCascadeUpdate(cascadeUpdate) {}
+	Row(ItemRow* data = nullptr)
+		: mData(data) {}
 
 	Row(const ItemRow* data)
-		: Row(const_cast<ItemRow*>(data), false)
-	{}
+		: mData(const_cast<ItemRow*>(data)), mConst(true) {}
 
 	Row(const Row& rhs);
 	Row& operator=(const Row& rhs);
 
-	void next();	///< make this row point to the next ItemRow
+	~Row();
 
-	/// When updating a value, you might want to change linked records as well
-	/// But not always.
-	void setCascadeUpdate(bool cascadeUpdate)
-	{
-		mCascadeUpdate = cascadeUpdate;
-	}
-	
-	void setCascadeDelete(bool cascadeDelete)
-	{
-		mCascadeDelete = cascadeDelete;
-	}
+	void next();	///< make this row point to the next ItemRow
 
 	struct const_iterator : public std::iterator<std::forward_iterator_tag, const Item>
 	{
@@ -565,8 +556,7 @@ class Row
 
 	ItemRow*	mData;
 	uint32_t	mLineNr = 0;
-	bool		mCascadeUpdate = true;
-	bool		mCascadeDelete = true;
+	bool		mConst = false;
 };
 
 // --------------------------------------------------------------------
@@ -590,10 +580,16 @@ struct AllConditionImpl : public ConditionImpl
 	virtual std::string str() const { return "ALL"; }
 };
 
+struct orConditionImpl;
+struct andConditionImpl;
+struct notConditionImpl;
+
 }
 
-struct Condition
+class Condition
 {
+  public:
+
 	Condition() : mImpl(nullptr) {}
 	Condition(detail::ConditionImpl* impl) : mImpl(impl) {}
 
@@ -635,6 +631,14 @@ struct Condition
 
 	bool empty() const		{ return mImpl == nullptr; }
 
+	friend Condition operator||(Condition&& a, Condition&& b);
+	friend Condition operator&&(Condition&& a, Condition&& b);
+
+	friend struct detail::orConditionImpl;
+	friend struct detail::andConditionImpl;
+	friend struct detail::notConditionImpl;
+
+  private:
 	detail::ConditionImpl*	mImpl;
 	bool mPrepared = false;
 };
@@ -1052,7 +1056,7 @@ inline Condition Not(Condition&& cond)
 // -----------------------------------------------------------------------
 // iterators
 
-template<typename RowType, typename ConditionType = void>
+template<typename RowType>
 class iterator_impl
 {
   public:
@@ -1065,12 +1069,12 @@ class iterator_impl
 	friend class Category;
 
 	iterator_impl(ItemRow* data) : mCurrent(data) {}
+	iterator_impl(const iterator_impl& i) : mCurrent(i.mCurrent) {}
 
-	template<typename T, std::enable_if_t<std::is_same_v<ConditionType,T>, int> = 0>
-	iterator_impl(ItemRow* data, Category& cat, const T& cond)
-		: mCurrent(data), mCat(&cat), mCondition(&cond)
+	iterator_impl& operator=(const iterator_impl& i)
 	{
-		skip();
+		mCurrent = i.mCurrent;
+		return *this;
 	}
 
 	virtual ~iterator_impl() = default;
@@ -1081,89 +1085,200 @@ class iterator_impl
 	iterator_impl& operator++()
 	{
 		mCurrent.next();
-		skip();
-
 		return *this;
 	}
 
-	iterator_impl operator++(int)					{ iterator_impl result(*this); this->operator++(); return result; } 
+	iterator_impl operator++(int)
+	{
+		iterator_impl result(*this);
+		this->operator++();
+		return result;
+	} 
 
 	bool operator==(const iterator_impl& rhs) const	{ return mCurrent == rhs.mCurrent; } 
 	bool operator!=(const iterator_impl& rhs) const	{ return not (mCurrent == rhs.mCurrent); } 
 
   private:
-
-	void skip()
-	{
-		if constexpr (not std::is_void_v<ConditionType>)
-		{
-			while (mCurrent and not (*mCondition)(*mCat, mCurrent))
-				mCurrent.next();
-		}
-	}
-
 	Row mCurrent;
-	Category* mCat = nullptr;
-	const Condition* mCondition = nullptr;
 };
 
-template<typename RowType>
-class conditional_iterator_proxy
-{
-  public:
-	using iterator = iterator_impl<RowType,Condition>;
-	using reference = typename iterator::reference;
+// template<typename RowType>
+// class conditional_iterator_proxy
+// {
+//   public:
 
-	conditional_iterator_proxy(ItemRow* head, Category& cat, Condition&& cond)
-		: mHead(head), mCat(cat), mCondition(std::forward<Condition>(cond))
-	{
-		mCondition.prepare(cat);
-	}
+	
 
-	iterator begin() const			{ return iterator(mHead, mCat, mCondition); }
-	iterator end() const			{ return iterator(nullptr, mCat, mCondition); }
+// 	using iterator = iterator_impl<RowType>;
+// 	using reference = typename iterator::reference;
 
-	bool empty() { return begin() == end(); }
-	size_t size() const { return std::distance(begin(), end()); }
+// 	conditional_iterator_proxy(ItemRow* head, Category& cat, Condition&& cond)
+// 		: mHead(head), mCat(cat), mCondition(std::forward<Condition>(cond))
+// 	{
+// 		mCondition.prepare(cat);
+// 	}
 
-	reference front() { return *begin(); }
+// 	conditional_iterator_proxy(conditional_iterator_proxy&& p);
 
-	Category& category() const		{ return mCat;}
+// 	conditional_iterator_proxy(const conditional_iterator_proxy&) = delete;
+// 	conditional_iterator_proxy& operator=(const conditional_iterator_proxy&) = delete;
 
-  private:
-	ItemRow* mHead;
-	Category& mCat;
-	Condition mCondition;
-};
+// 	iterator begin() const			{ return iterator(mHead, mCat, mCondition); }
+// 	iterator end() const			{ return iterator(nullptr, mCat, mCondition); }
+
+// 	bool empty() { return begin() == end(); }
+// 	size_t size() const { return std::distance(begin(), end()); }
+
+// 	reference front() { return *begin(); }
+
+// 	Category& category() const		{ return mCat;}
+
+//   private:
+// 	bool mAtEnd = false;
+// 	ItemRow* mCurrent;
+// 	Category& mCat;
+// 	Condition mCondition;
+// };
 
 // --------------------------------------------------------------------
 // class RowSet is used to return find results. Use it to re-order the results
 // or to group them 
 
-class RowSet : public vector<Row>
+class RowSet
 {
 	typedef vector<Row>	base_type;
 
   public:
+
+	using size_type = std::size_t;
+    using difference_type = std::ptrdiff_t;
+
+	RowSet(Category& cat);
+	RowSet(Category& cat, Condition&& cond);
 	RowSet(const RowSet& rhs);
 	RowSet(RowSet&& rhs);
-	RowSet(conditional_iterator_proxy<Row>&& results);
+	virtual ~RowSet();
 
 	RowSet& operator=(const RowSet& rhs);
 	RowSet& operator=(RowSet&& rhs);
 
-	RowSet& operator=(conditional_iterator_proxy<Row>& results);
-	RowSet& operator=(conditional_iterator_proxy<const Row>& results);
-
-	RowSet(Category& cat);
-	
 	RowSet& orderBy(const string& Item)
 		{ return orderBy({ Item }); }
 	
 	RowSet& orderBy(std::initializer_list<string> Items);
 
+	class iterator
+	{
+	  public:
+		using iterator_category = std::forward_iterator_tag;
+		using value_type = Row;
+		using difference_type = std::ptrdiff_t;
+		using pointer = value_type*;
+		using reference = value_type&;
+
+		iterator() {}
+
+		iterator(const iterator& i)
+			: mCurrentIter(i.mCurrentIter)
+			, mCurrentRow(i.mCurrentRow) {}
+
+		iterator(const std::vector<ItemRow*>::iterator& i)
+			: mCurrentIter(i)
+			, mCurrentRow(*i) {}
+
+		iterator& operator=(const iterator& i)
+		{
+			mCurrentIter = i.mCurrentIter;
+			mCurrentRow = i.mCurrentRow;
+			return *this;
+		}
+
+		reference operator*()		{ return mCurrentRow; }
+		pointer operator->()		{ return &mCurrentRow; }
+
+		iterator& operator++()
+		{
+			++mCurrentIter;
+			mCurrentRow = Row(*mCurrentIter);
+			return *this;
+		}
+
+		iterator operator++(int)
+		{
+			iterator t(*this);
+			operator++();
+			return t;
+		}
+
+		iterator& operator+=(difference_type i)
+		{
+			while (i-- > 0) operator++();
+			return *this;
+		}
+
+		iterator operator+(difference_type i) const
+		{
+			auto result = *this;
+			result += i;
+			return result;
+		}
+
+		friend iterator operator+(difference_type i, const iterator& iter)
+		{
+			auto result = iter;
+			result += i;
+			return result;
+		}
+
+		friend difference_type operator-(const iterator& a, const iterator& b)
+		{
+			return std::distance(a.mCurrentIter, b.mCurrentIter);
+		}
+
+		bool operator==(const iterator& i) const		{ return mCurrentIter == i.mCurrentIter; }
+		bool operator!=(const iterator& i) const		{ return mCurrentIter != i.mCurrentIter; }
+
+	  private:
+
+		friend class RowSet;
+
+		std::vector<ItemRow*>::iterator current() const	{ return mCurrentIter; }
+
+		std::vector<ItemRow*>::iterator mCurrentIter;
+		Row mCurrentRow;
+	};
+
+	iterator begin()		{ return iterator(mItems.begin()); }
+	iterator end()			{ return iterator(mItems.end()); }
+
+	Row front() const		{ return Row(mItems.front()); }
+	size_t size() const		{ return mItems.size(); }
+	bool empty() const		{ return mItems.empty(); }
+
+	template<typename InputIterator>
+	iterator insert(iterator pos, InputIterator b, InputIterator e)
+	{
+		difference_type offset = pos - begin();
+		for (auto i = b; i != e; ++i, ++offset)
+			insert(begin() + offset, *i);
+		return begin() + offset;
+	}
+
+	iterator insert(iterator pos, Row& row)
+	{
+		return insert(pos, row.mData);
+	}
+
+	iterator insert(iterator pos, ItemRow* item)
+	{
+		return iterator(mItems.insert(pos.current(), item));
+	}
+
   private:
-	Category*	mCat;
+	Category*			mCat;
+	vector<ItemRow*>	mItems;
+
+	// Condition*	mCond;
 };
 
 // --------------------------------------------------------------------
@@ -1205,9 +1320,9 @@ class Category
 	
 	Row operator[](Condition&& cond);
 
-	conditional_iterator_proxy<Row> find(Condition&& cond)
+	RowSet find(Condition&& cond)
 	{
-		return conditional_iterator_proxy<Row>(mHead, *this, std::forward<Condition>(cond));
+		return RowSet(*this, std::forward<Condition>(cond));
 	}
 
 	bool exists(Condition&& cond) const;

src/Cif++.cpp

@@ -1107,29 +1107,50 @@ size_t CatIndex::size() const
 
 // --------------------------------------------------------------------
 
+RowSet::RowSet(Category& cat)
+	: mCat(&cat)
+	// , mCond(nullptr)
+{
+}
+
+RowSet::RowSet(Category& cat, Condition&& cond)
+	: mCat(&cat)
+	// , mCond(new Condition(std::forward<Condition>(cond)))
+{
+	cond.prepare(cat);
+
+	for (auto r: cat)
+	{
+		if (cond(cat, r))
+			mItems.push_back(r.mData);
+	}
+}
+
 RowSet::RowSet(const RowSet& rhs)
-	: base_type(rhs)
-	, mCat(rhs.mCat)
+	: mCat(rhs.mCat)
+	, mItems(rhs.mItems)
+	// , mCond(nullptr)
 {
 }
 
 RowSet::RowSet(RowSet&& rhs)
-	: base_type(move(rhs))
-	, mCat(rhs.mCat)
+	: mCat(rhs.mCat)
+	, mItems(std::move(rhs.mItems))
+	// , mCond(rhs.mCond)
 {
+	// rhs.mCond = nullptr;
 }
 
-RowSet::RowSet(conditional_iterator_proxy<Row>&& results)
-	: vector<Row>(results.begin(), results.end())
-	, mCat(&results.category())
+RowSet::~RowSet()
 {
+	// delete mCond;
 }
 
 RowSet& RowSet::operator=(const RowSet& rhs)
 {
 	if (this != &rhs)
 	{
-		base_type::operator=(rhs);
+		mItems = rhs.mItems;
 		mCat = rhs.mCat;
 	}
 	
@@ -1140,37 +1161,18 @@ RowSet& RowSet::operator=(RowSet&& rhs)
 {
 	if (this != &rhs)
 	{
-		base_type::operator=(move(rhs));
+		std::swap(mItems, rhs.mItems);
 		mCat = rhs.mCat;
 	}
 	
 	return *this;
 }
 
-RowSet& RowSet::operator=(conditional_iterator_proxy<Row>& results)
-{
-	clear();
-	insert(begin(), results.begin(), results.end());
-	return *this;
-}
-
-RowSet& RowSet::operator=(conditional_iterator_proxy<const Row>& results)
-{
-	clear();
-	insert(begin(), results.begin(), results.end());
-	return *this;
-}
-
-RowSet::RowSet(Category& cat)
-	: mCat(&cat)
-{
-}
-
 RowSet& RowSet::orderBy(initializer_list<string> items)
 {
 	RowComparator c(mCat, items.begin(), items.end());
 	
-	stable_sort(begin(), end(), c);
+	stable_sort(mItems.begin(), mItems.end(), c);
 	
 	return *this;
 }
@@ -1294,6 +1296,21 @@ size_t Category::addColumn(const string& name)
 	return result;
 }
 
+// RowSet Category::find(Condition&& cond)
+// {
+// 	RowSet result(*this);
+
+// 	cond.prepare(*this);
+
+// 	for (auto r: *this)
+// 	{
+// 		if (cond(*this, r))
+// 			result.push_back(r);
+// 	}
+
+// 	return result;
+// }
+
 void Category::reorderByIndex()
 {
 	if (mIndex != nullptr)
@@ -1564,7 +1581,7 @@ size_t Category::erase(Condition&& cond, std::function<void(const Row&)>&& verbo
 
 void Category::eraseOrphans(Condition&& cond)
 {
-	RowSet remove(*this);
+	vector<ItemRow*> remove;
 	
 	cond.prepare(*this);
 
@@ -1577,12 +1594,12 @@ void Category::eraseOrphans(Condition&& cond)
 					 << r << endl
 					 << endl;
 
-			remove.push_back(r);
+			remove.push_back(r.mData);
 		}
 	}
 
 	for (auto r: remove)
-		erase(r);
+		erase(iterator(r));
 }
 
 void Category::erase(Row r)
@@ -2228,10 +2245,14 @@ void Category::write(ostream& os, const vector<string>& columns)
 
 Row::Row(const Row& rhs)
 	: mData(rhs.mData)
-	, mCascadeUpdate(rhs.mCascadeUpdate)
 {
 }
 
+Row::~Row()
+{
+	
+}
+
 void Row::next()
 {
 	if (mData != nullptr)
@@ -2241,12 +2262,14 @@ void Row::next()
 Row& Row::operator=(const Row& rhs)
 {
 	mData = rhs.mData;
-	mCascadeUpdate = rhs.mCascadeUpdate;
 	return *this;
 }
 
 void Row::assign(const std::vector<Item>& values)
 {
+	if (mConst)
+		throw logic_error("Row is const, cannot assign values");
+
 	auto cat = mData->mCategory;
 
 	map<string,tuple<int,string,string>> changed;
@@ -2264,47 +2287,45 @@ void Row::assign(const std::vector<Item>& values)
 
 	// see if we need to update any child categories that depend on these values
 	// auto iv = col.mValidator;
-	if (mCascadeUpdate)
+
+	auto& validator = cat->getValidator();
+	auto& db = cat->db();
+
+	for (auto linked: validator.getLinksForParent(cat->mName))
 	{
-		auto& validator = cat->getValidator();
-		auto& db = cat->db();
+		auto childCat = db.get(linked->mChildCategory);
+		if (childCat == nullptr)
+			continue;
 
-		for (auto linked: validator.getLinksForParent(cat->mName))
-		{
-			auto childCat = db.get(linked->mChildCategory);
-			if (childCat == nullptr)
-				continue;
+		// if (find(linked->mParentKeys.begin(), linked->mParentKeys.end(), iv->mTag) == linked->mParentKeys.end())
+		// 	continue;
 
-			// if (find(linked->mParentKeys.begin(), linked->mParentKeys.end(), iv->mTag) == linked->mParentKeys.end())
-			// 	continue;
+		Condition cond;
+		string childTag;
 
-			Condition cond;
-			string childTag;
+		vector<Item> newValues;
+		
+		for (size_t ix = 0; ix < linked->mParentKeys.size(); ++ix)
+		{
+			string pk = linked->mParentKeys[ix];
+			string ck = linked->mChildKeys[ix];
 
-			vector<Item> newValues;
-			
-			for (size_t ix = 0; ix < linked->mParentKeys.size(); ++ix)
+			if (changed.count(pk) > 0)
 			{
-				string pk = linked->mParentKeys[ix];
-				string ck = linked->mChildKeys[ix];
-
-				if (changed.count(pk) > 0)
-				{
-					childTag = ck;
-					cond = move(cond) && (Key(ck) == std::get<1>(changed[pk]));
-					newValues.emplace_back(ck, std::get<2>(changed[pk]));
-				}
-				else
-				{
-					const char* value = (*this)[pk].c_str();
-					cond = move(cond) && (Key(ck) == value);
-				}
+				childTag = ck;
+				cond = move(cond) && (Key(ck) == std::get<1>(changed[pk]));
+				newValues.emplace_back(ck, std::get<2>(changed[pk]));
+			}
+			else
+			{
+				const char* value = (*this)[pk].c_str();
+				cond = move(cond) && (Key(ck) == value);
 			}
-
-			auto rows = childCat->find(move(cond));
-			for (auto& cr: rows)
-				cr.assign(newValues);
 		}
+
+		auto rows = childCat->find(move(cond));
+		for (auto& cr: rows)
+			cr.assign(newValues);
 	}
 }
 
@@ -2332,6 +2353,9 @@ void Row::assign(size_t column, const string& value, bool skipUpdateLinked)
 	if (mData == nullptr)
 		throw logic_error("invalid Row, no data assigning value '" + value + "' to column with index " + to_string(column));
 	
+	if (mConst)
+		throw logic_error("Row is const, cannot assign value '" + value + "' to column with index " + to_string(column));
+
 	auto cat = mData->mCategory;
 	auto& col = cat->mColumns[column];
 
@@ -2416,7 +2440,7 @@ void Row::assign(size_t column, const string& value, bool skipUpdateLinked)
 
 	// see if we need to update any child categories that depend on this value
 	auto iv = col.mValidator;
-	if (not skipUpdateLinked and iv != nullptr and mCascadeUpdate)
+	if (not skipUpdateLinked and iv != nullptr)
 	{
 		auto& validator = cat->getValidator();
 		auto& db = cat->db();

src/Symmetry.cpp

@@ -153,9 +153,11 @@ int32_t GetRotationalIndexNumber(int spacegroup, const clipper::RTop_frac& rt)
 
 	SymopData k
 	{
-		rte(0, 0), rte(0, 1), rte(0, 2),
-		rte(1, 0), rte(1, 1), rte(1, 2),
-		rte(2, 0), rte(2, 1), rte(2, 2)
+		{
+			rte(0, 0), rte(0, 1), rte(0, 2),
+			rte(1, 0), rte(1, 1), rte(1, 2),
+			rte(2, 0), rte(2, 1), rte(2, 2)
+		}
 	};
 
 	for (int i = 0; i < 3; ++i)

test/unit-test.cpp

@@ -56,4 +56,40 @@ _test.name
   	});
 
 	BOOST_CHECK_EQUAL(n, 1);
+}
+
+// --------------------------------------------------------------------
+
+BOOST_AUTO_TEST_CASE(ut2)
+{
+	using namespace mmcif;
+
+	auto f = R"(data_TEST
+#
+loop_
+_test.id
+_test.name
+1 aap
+2 noot
+3 mies
+	)"_cf;
+
+	auto& db = f.firstDatablock();
+
+	BOOST_CHECK(db.getName() == "TEST");
+	
+	auto& test = db["test"];
+	BOOST_CHECK(test.size() == 3);
+
+	int n = 0;
+	for (auto r: test.find(cif::Key("name") == "aap"))
+	{
+		BOOST_CHECK(++n == 1);
+		BOOST_CHECK(r["id"].as<int>() == 1);
+		BOOST_CHECK(r["name"].as<std::string>() == "aap");
+	}
+
+	auto t = test.find(cif::Key("id") == 1);
+	BOOST_CHECK(not t.empty());
+	BOOST_CHECK(t.front()["name"].as<std::string>() == "aap");
 }
\ No newline at end of file

tools/symop-map-generator.cpp

@@ -406,10 +406,10 @@ struct SymopDataBlock
 			spacegroupNr = sp;
 
 			cout << "    { " << setw(3) << sp
-					<< ", " << setw(3) << o << ", { ";
+					<< ", " << setw(3) << o << ", { { ";
 			for (auto i: get<2>(sd))
 				cout << setw(2) << i << ',';
-			cout << " } }," << endl;
+			cout << " } } }," << endl;
 		}
 
 		cout << "};" << endl;