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 <brit...@apache.org> wrote:

> Hello Robert,
>
> Robert Huffman <robert.huff...@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 <brit...@apache.org>
> > wrote:
> >
> >> <moving this to the dev ML>
> >>
> >> Robert Huffman <robert.huff...@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 <brit...@apache.org>
> >>> wrote:
> >>>
> >>> > Hello Robert,
> >>> >
> >>> > Robert Huffman <robert.huff...@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
> >>> >
> >>>
> >>
> >
>

Reply via email to