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 >>> > >>> >> >
