From: Warren Young
Date: November 8 2008 5:11am
Subject: Re: insertfrom( ) questions
List-Archive: http://lists.mysql.com/plusplus/8123
Message-Id: <49151F76.7020003@etr-usa.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Rick Gutleber wrote:
> 
> 1.  Does the new insertfrom( ) method and the InsertPolicy and SQLStream 
> objects require anything new in the tests?  The tests are very concise; 
> I don't know if they are meant to be exhaustive.

The MySQL++ test suite's concision reflects its immaturity, not a design 
goal.  There should be a test for SQLStream at least in tests/* and code 
to exercise insertfrom() in examples/*.

For the example, maybe something that reads a tab-delimited file of 
'stock' items, makes a vector of SSQLSes from this, then inserts that 
into the DB?

> 2.  Do I need to make additions to the user manual to reflect these new 
> pieces?  Or is there someone else who handles documentation changes?

Leave userman to me.

> I attached the patch but if there is anything else I need to do, I will 
> make those changes and resubmit.

Yeah, it needs some style fixes.

1. I don't see that splitting insertfrom() into public and private 
halves buys much.  Multiple 'success = false' lines isn't any less 
redundant than multiple 'ac.rollback()' lines.  (Not that I think you'll 
have too many method calls on 'ac'...most of them should be at the top 
and bottom of the function.)  You also end up with an empty transaction 
if the insert range is empty, due to starting the transaction way up 
high like that, before you test the iterator range.  If I'm wrong, tell 
me why.

2. In the code, use tabs instead of spaces, size 4 [*], and lint your 
patch for other pointless whitespace changes.  This accounts for about 
half your patch.  As far as I can tell, 100% of the changes to 
lib/query.cpp are of this type, but it happens all through the patch. 
As a result, I haven't really studied the patch carefully.  Too much 
noise masking the signal.

3. Take the unistd.h out of examples/cpool.cpp.  I fixed that problem in 
svn a different way.  Besides, it's unrelated to this patch.  (Reading 
through your patch before submitting it would have caught #2, too.)

4. Say 'svn add lib/sqlstream.* lib/insertpolicy.h' before making the 
next patch.  These files are missing from the one you just sent.

5. Use 0 instead of NULL.  (Just style.)

6. Don't cuddle 'else'.

[*] Yes, we do use spaces instead of tabs in all of the documentation 
files.  I've configured my text editor to switch whitespace modes based 
on the file type, so I don't often notice the change any more.  We may 
at some point go with spaces across the board, but for the nonce, follow 
the current convention.