commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Sicker <boa...@gmail.com>
Subject Re: [dbutils] Would it be possible to have parameters passed to QueryRunner as an Iterable?
Date Wed, 22 Jun 2016 17:03:48 GMT
GitHub mirrors are mainly available to make it easier for users to
contribute pull requests instead of patches.

On 22 June 2016 at 10:59, Robert Huffman <robert.huffman@gmail.com> wrote:

> Got it. Thanks.
>
> On Wed, Jun 22, 2016 at 8:12 AM, Gary Gregory <garydgregory@gmail.com>
> wrote:
>
> > Nothing is moving to GitHub. Some projects are moving from Svn to Git
> > within Apache and then being _mirrored_ to GitHub.
> >
> > Gary
> >
> > On Wed, Jun 22, 2016 at 8:10 AM, Robert Huffman <
> robert.huffman@gmail.com>
> > wrote:
> >
> > > Some time ago I opened an issue for this and attached a patch. I see
> that
> > > some of the other Commons projects are moving to GitHub. Any chance
> that
> > > will happen for dbutils?
> > >
> > >
> > >
> > > On Fri, May 20, 2016 at 5:36 AM, Benedikt Ritter <britter@apache.org>
> > > wrote:
> > >
> > > > Hello Robert,
> > > >
> > > > Robert Huffman <robert.huffman@gmail.com> schrieb am Do., 19. Mai
> 2016
> > > um
> > > > 17:00 Uhr:
> > > >
> > > > > Thanks for the comments. Yes, it is in progress.
> > > > >
> > > > > I generally keep imports collapsed and never look at them. I hadn't
> > > even
> > > > > realized that was the style here. Thanks for pointing it out.
> > > > >
> > > > > I already changed ITERABLE to COLLECTION. (Originally I thought I
> > would
> > > > be
> > > > > using Iterable) and removed the parameterType from the signature.
> > > > >
> > > > > I agree with you: I'm not so sure using JUnit parameterized tests
> is
> > a
> > > > > great idea here. It is unfortunate JUnit doesn't give you more
> > control.
> > > > > Other frameworks (like Spock or TestNG) let you parameterize
> specific
> > > > test
> > > > > methods intead of the entire class. I am thinking that once I fix
> > > execute
> > > > > and update tests to also use the parameter type, most tests in the
> > > > > QueryRunnerTest will use it, so perhaps it won't be too bad.
> > > > >
> > > > > If you don't like it, what alternative would you suggest? I'm not
> > crazy
> > > > > about the idea of just copying the test methods. I suppose I could
> > just
> > > > add
> > > > > calls with collections to the existing test methods, but I'm not
> > crazy
> > > > > about that, either.
> > > > >
> > > >
> > > > It's okay for me if in the end all (or most of) the test methods
> > actually
> > > > use the test parameters. If not, I suggest splitting the test up into
> > to
> > > > test classes.
> > > >
> > > > Regards,
> > > > Benedikt
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <
> britter@apache.org
> > >
> > > > > wrote:
> > > > >
> > > > >> <moving this to the dev ML>
> > > > >>
> > > > >> Robert Huffman <robert.huffman@gmail.com> schrieb am Do.,
19. Mai
> > > 2016
> > > > >> um 15:32 Uhr:
> > > > >>
> > > > >>> Actually, Iterable doesn't work: in fillStatement you need
to
> know
> > > the
> > > > >>> size
> > > > >>> and that's not easily obtainable from an Iterable. So I'm
using
> > > > >>> Collection
> > > > >>> instead.
> > > > >>>
> > > > >>> I cloned the project and have QueryRunner.query working already.
> > > > >>> Basically
> > > > >>> I changed the private query method to take a Collection instead
> of
> > > > >>> varargs
> > > > >>> for the parameters. A public method is added that takes a
> > Collection,
> > > > and
> > > > >>> the public methods that take arrays simply use Arrays.asList
on
> the
> > > > >>> arrays.  You can check out the approach here:
> > > > >>>
> > > > >>>
> > > > >>>
> > > >
> > >
> >
> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
> > > > >>>
> > > > >>> I will take the same approach on update, insert and batch,
> create a
> > > > JIRA
> > > > >>> and attach a patch.
> > > > >>>
> > > > >>
> > > > >> I've added a few comments. Looks good to me overall.
> > > > >>
> > > > >>
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <
> > britter@apache.org
> > > >
> > > > >>> wrote:
> > > > >>>
> > > > >>> > Hello Robert,
> > > > >>> >
> > > > >>> > Robert Huffman <robert.huffman@gmail.com> schrieb
am Mi., 18.
> > Mai
> > > > >>> 2016 um
> > > > >>> > 19:00 Uhr:
> > > > >>> >
> > > > >>> > > If a prepared statement is built dynamically, with
a variable
> > > > number
> > > > >>> of
> > > > >>> > > parameters, and parameters are collected in a Collection
of
> > some
> > > > sort
> > > > >>> > > instead of an array, usage QueryRunner requires
that the
> > > collection
> > > > >>> be
> > > > >>> > > converted to an array first. This means the parameters
are
> > > iterated
> > > > >>> > twice:
> > > > >>> > > once to convert to an array and once again in
> > > > >>> QueryRunner.fillStatement.
> > > > >>> > >
> > > > >>> > > Would it violate a design decision if methods were
added to
> > > > >>> QueryRunner
> > > > >>> > > that took the parameters as an Iterable instead
of as varags?
> > It
> > > > >>> should
> > > > >>> > be
> > > > >>> > > straightforward to add such methods and use an
Iterable
> wrapper
> > > > >>> around an
> > > > >>> > > array to have the varargs methods invoke the new
methods that
> > > take
> > > > >>> > > Iterables.
> > > > >>> > >
> > > > >>> > > I would be happy to submit a patch if this does
not violate
> > some
> > > > >>> sort of
> > > > >>> > > design decision I am not aware of and if the implementation
> > > > approach
> > > > >>> > sounds
> > > > >>> > > reasonable.
> > > > >>> > >
> > > > >>> >
> > > > >>> > Sounds like a reasonable addition, although I'm not
sure I
> > > understand
> > > > >>> your
> > > > >>> > proposal with the "Iterable wrapper around an array".
Feel free
> > to
> > > > >>> create a
> > > > >>> > JIRA and provide a patch/github PR for adding this
> functionality.
> > > > >>> Further
> > > > >>> > design discussions about this addition should go to
the dev
> > mailing
> > > > >>> list.
> > > > >>> >
> > > > >>> > Benedikt
> > > > >>> >
> > > > >>>
> > > > >>
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > E-Mail: garydgregory@gmail.com | ggregory@apache.org
> > Java Persistence with Hibernate, Second Edition
> > <http://www.manning.com/bauer3/>
> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > Spring Batch in Action <http://www.manning.com/templier/>
> > Blog: http://garygregory.wordpress.com
> > Home: http://garygregory.com/
> > Tweet! http://twitter.com/GaryGregory
> >
>



-- 
Matt Sicker <boards@gmail.com>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message