List:MySQL++« Previous MessageNext Message »
From:Warren Young Date:June 22 2005 2:15am
Subject:Re: RFC: Row::operator[] change
View as plain text  
I just finished re-doing this patch, and it's substantially different 
from my previous effort:

- Row's lookup-by-index function is now at(), not col_name(), to work 
more like the Standard Library.

- I decided not to remove const_subscript_container as Row's superclass, 
but instead change const_subscript_container and all its subclasses 
(Row, Fields, and Result) provide at() instead of operator[](int).

- This version is a bit more comprehensive.

- This one applies to the new v2.0 alpha, so it's necessarily somewhat 
different than my effort from last April.


So, last time I offered a patch on this subject for comment, the only 
objections were that some people were using index-by-int for speed 
reasons.  I pointed out that you can still use at() (nee' col_num()) for 
that.

Could those of you still interested in this try this patch against the 
current svn version, and see what you think?  Notice how many files this 
touches: it's fairly intrusive.  That's why I'm running this RFC again, 
instead of checking it in.

Personally, I think it's good to have the option of indexing by name, 
and find indexing by integer less compelling.  If speed is your thing, I 
think you should be using SSQLS instead anyway.

Index: doc/userman/userman.xml
===================================================================
--- doc/userman/userman.xml	(revision 812)
+++ doc/userman/userman.xml	(working copy)
@@ -3006,6 +3006,17 @@
 			<computeroutput>value_list</computeroutput>,
 			without any thought of whether it made
 			sense.</para>
+
+			<para>Row objects can now be indexed by a
+			string again (e.g. row["myfield"]) as in
+			v1.7.9; lookup_by_name() was removed. The
+			underlying problem that lead to that change
+			still exists, so this is at the expense of
+			indexing by an integer. To get a field by
+			its index, use the new at() function, by
+			analogy with vector::at(). This change was
+			done because it's generally more useful to
+			index a Row by a field name.</para>
 		</sect3>
 	</sect2>
 
Index: lib/fields.cpp
===================================================================
--- lib/fields.cpp	(revision 805)
+++ lib/fields.cpp	(working copy)
@@ -35,7 +35,7 @@
 	return res->num_fields();
 }
 
-const Field & Fields::operator[] (Fields::size_type i) const
+const Field& Fields::at(Fields::size_type i) const
 {
 	res->field_seek(i);
 	return res->fetch_field();
Index: lib/row.h
===================================================================
--- lib/row.h	(revision 808)
+++ lib/row.h	(working copy)
@@ -469,11 +469,10 @@
 	/// \brief Get the number of fields in the row.
 	size_type size() const;
 
-	/// \brief Get the value of a field given its index.
+	/// \brief Get the value of a field given its name.
 	///
-	/// If the array index is out of bounds, the C++ standard says that
-	/// the underlying vector container should throw an exception.
-	/// Whether it actually does is probably implementation-dependent.
+	/// If the field does not exist in this row, we throw a BadFieldName
+	/// exception.
 	///
 	/// Note that we return the
 	/// \link mysqlpp::ColData_Tmpl ColData \endlink object by value.
@@ -483,30 +482,33 @@
 	/// like this:
 	///
 	/// \code
-	///  string s = row[2];
+	///  string s = row["myfield"];
 	/// \endcode
 	///
-	/// That accesses the third field in the row, returns a temporary
+	/// That accesses myfield within the row, returns a temporary
 	/// ColData object, which is then automatically converted to a
 	/// \c std::string and copied into \c s.  That works fine, but
 	/// beware of this similar but incorrect construct:
 	///
 	/// \code
-	///  const char* pc = row[2];
+	///  const char* pc = row["myfield"];
 	/// \endcode
 	///
 	/// This one line of code does what you expect, but \c pc is then a
 	/// dangling pointer: it points to memory owned by the temporary
 	/// ColData object, which will have been destroyed by the time you
 	/// get around to actually \e using the pointer.
-	const ColData operator [](size_type i) const;
+	///
+	/// This function is rather inefficient.  If that is a concern for
+	/// you, use at() or the SSQLS mechanism instead.
+	const ColData operator [](const char* field) const;
 
-	/// \brief Get the value of a field given its field name.
+	/// \brief Get the value of a field given its index.
 	///
-	/// This function is rather inefficient.  You should use operator[]
-	/// if you're using Rows directly, or SSQLS for efficient named
-	/// access to row elements.
-	const ColData lookup_by_name(const char*) const;
+	/// If the index is out-of-bounds, the underlying vector is supposed
+	/// to throw an exception according to the C++ Standard.  Whether it
+	/// actually does this is implementation-dependent.
+	const ColData at(size_type i) const;
 
 	/// \brief Return the value of a field given its index, in raw form.
 	///
Index: lib/result.h
===================================================================
--- lib/result.h	(revision 805)
+++ lib/result.h	(working copy)
@@ -304,7 +304,7 @@
 	/// \brief Get the underlying Field structure given its index.
 	const Field& fields(unsigned int i) const
 	{
-		return fields_[i];
+		return fields_.at(i);
 	}
 	
 	/// \brief Returns true if the other ResUse object shares the same
@@ -412,13 +412,15 @@
 	{
 		return size_type(num_rows());
 	}
+
 	/// \brief Alias for num_rows(), only with different return type.
 	size_type rows() const
 	{
 		return size_type(num_rows());
 	}
+
 	/// \brief Get the row with an offset of i.
-	const Row operator [](size_type i) const
+	const Row at(size_type i) const
 	{
 		data_seek(i);
 		return fetch_row();
Index: lib/custom.pl
===================================================================
--- lib/custom.pl	(revision 805)
+++ lib/custom.pl	(working copy)
@@ -262,7 +262,7 @@
 	$parm_simple2c_b .= ", " unless $j == $i;
 	$defs  .= "    T$j I$j;";
 	$defs  .= "\n" unless $j == $i;
-	$popul .= "    s->I$j = static_cast<T$j>(row[ O$j ]);";
+	$popul .= "    s->I$j = static_cast<T$j>(row.at(O$j));";
 	$popul .= "\n" unless $j == $i;
         $names .= "    N$j ";
 	$names .= ",\n" unless $j == $i;
Index: lib/field_names.cpp
===================================================================
--- lib/field_names.cpp	(revision 805)
+++ lib/field_names.cpp	(working copy)
@@ -39,7 +39,7 @@
 	reserve(num);
 
 	for (int i = 0; i < num; i++) {
-		std::string p(res->fields()[i].name);
+		std::string p(res->fields().at(i).name);
 		str_to_lwr(p);
 		push_back(p);
 	}
Index: lib/row.cpp
===================================================================
--- lib/row.cpp	(revision 805)
+++ lib/row.cpp	(working copy)
@@ -35,7 +35,7 @@
 	return res_->num_fields();
 }
 
-const ColData Row::operator [](size_type i) const
+const ColData Row::at(size_type i) const
 {
 	if (!initialized_) {
 		if (throw_exceptions())
@@ -47,14 +47,14 @@
 	return ColData(data_.at(i).c_str(), res_->types(i), is_nulls_[i]);
 }
 
-const ColData Row::lookup_by_name(const char* i) const
+const ColData Row::operator [](const char* field) const
 {
-	int si = res_->field_num(std::string(i));
-	if (si < res_->num_fields()) {
-		return (*this)[si];
+	int si = res_->field_num(std::string(field));
+	if (si < size()) {
+		return at(si);
 	}
 	else {
-		throw BadFieldName(i);
+		throw BadFieldName(field);
 	}
 }
 
Index: lib/fields.h
===================================================================
--- lib/fields.h	(revision 805)
+++ lib/fields.h	(working copy)
@@ -54,12 +54,12 @@
 	}
 	
 	/// \brief Returns a field given its index.
-	const Field& operator [](size_type i) const;
+	const Field& at(size_type i) const;
 
 	/// \brief Returns a field given its index.
-	const Field& operator [](int i) const
+	const Field& at(int i) const
 	{
-		return operator [](size_type(i));
+		return at(static_cast<size_type>(i));
 	}
 
 	size_type size() const;	///< get the number of fields
Index: lib/resiter.h
===================================================================
--- lib/resiter.h	(revision 805)
+++ lib/resiter.h	(working copy)
@@ -82,7 +82,7 @@
 	virtual size_type size() const = 0;	
 
 	/// \brief Return element at given index in container
-	virtual ReturnType operator [](SizeType i) const = 0;
+	virtual ReturnType at(SizeType i) const = 0;
 
 	/// \brief Return maximum number of elements that can be stored
 	/// in container without resizing.
@@ -182,11 +182,11 @@
 	
 	/// \brief Access the current pointed-to element within the
 	/// container
-	ReturnType* operator ->() const { return &((*d)[i]); }
+	ReturnType* operator ->() const { return &(d->at(i)); }
 	
 	/// \brief Dereference the iterator, returning the pointed-to
 	/// element within the container.
-	ReturnType operator *() const { return (*d)[i]; }
+	ReturnType operator *() const { return d->at(i); }
 	
 	/// \brief Return the an element in the container given its index
 	ReturnType operator [](SizeType n) const { return (*d)[n]; }
Index: Wishlist
===================================================================
--- Wishlist	(revision 808)
+++ Wishlist	(working copy)
@@ -60,13 +60,6 @@
 
 	o Fold RowTemplate class into Row.
 
-	o Swap the definitions of Row::operator[] and lookup_by_name().
-	  That is, have the operator take a field name, and define a
-	  function (at()?) that takes a field index.  The original problem
-	  solved in 1.7.10 still exists, in that modern compilers won't
-	  let you overload the operator on both strings and integers,
-	  but the string form probably is the most useful.
-
 	o Decide on a new soname scheme?  It'd be nice if the soname had
 	  some obvious relationship to the library version.  We could
 	  actually go with a 1:1 relationship, and simply override the
Index: examples/dbinfo.cpp
===================================================================
--- examples/dbinfo.cpp	(revision 805)
+++ examples/dbinfo.cpp	(working copy)
@@ -133,7 +133,7 @@
 				for (counter = 0; counter < columns; counter++) {
 					if (widths[counter]) {
 						cout << ' ' << setw(widths[counter]) <<
-								row[counter] << ' ';
+								row.at(counter) << ' ';
 					}
 				}
 				cout << endl;
@@ -157,7 +157,7 @@
 		for (i = res.begin(); i != res.end(); ++i) {
 			row = *i;
 			for (int counter = 0; counter < columns; counter++) {
-				cout << row[counter] << "  ";
+				cout << row.at(counter) << "  ";
 			}
 			cout << endl;
 		}
Index: examples/util.cpp
===================================================================
--- examples/util.cpp	(revision 812)
+++ examples/util.cpp	(working copy)
@@ -134,8 +134,15 @@
 	// specially.  (See Row::operator[]'s documentation for the reason.)
 	// As for the rest of the fields, Row::operator[] returns a ColData
 	// object, which can convert itself to any standard C type.
-	string item(row[0]);
-	print_stock_row(item, row[1], row[2], row[3], row[4]);
+	//
+	// We index the row by field name to demonstrate the feature, and
+	// also because it makes the code more robust in the face of schema
+	// changes.  Use Row::at() instead if efficiency is paramount.  To
+	// maintain efficiency while keeping robustness, use the SSQLS
+	// feature, demoed in the custom* examples.
+	string item(row["item"]);
+	print_stock_row(item, row["num"], row["weight"], row["price"],
+			row["sdate"]);
 }
 
 
Index: examples/custom3.cpp
===================================================================
--- examples/custom3.cpp	(revision 805)
+++ examples/custom3.cpp	(working copy)
@@ -71,7 +71,7 @@
 		// there's no point in storing the result in an STL container.
 		// We can store the first row directly into a stock structure
 		// because one of an SSQLS's constructors takes a Row object.
-		stock row = res[0];
+		stock row = res.at(0);
 
 		// Create a copy so that the replace query knows what the
 		// original values are.

Thread
RFC: Row::operator[] changeWarren Young5 Apr
  • Re: RFC: Row::operator[] changeEarl Miles5 Apr
    • Re: RFC: Row::operator[] changeWarren Young5 Apr
  • Re: RFC: Row::operator[] changeChris Frey5 Apr
    • Re: RFC: Row::operator[] changeWarren Young22 Jun
  • Re: RFC: Row::operator[] changeWarren Young22 Jun
    • Re: RFC: Row::operator[] changeWarren Young25 Jun
Re: RFC: Row::operator[] changeBruce Keats11 Apr