I am +1 for making it take Iterable. Pretty much everything could want
to pass in implements Iterable:
http://docs.oracle.com/javase/6/docs/api/java/lang/Iterable.html
I am +1/2 on making the change to {with,add}AllInput. I quite like the
name addAll since it's similar to the java collection method names,
but we do have two distinct types, input and output.
Brock
On Sat, Oct 6, 2012 at 11:01 AM, James Kinley <[email protected]> wrote:
> Dave,
>
> I really don't mind either way - lets see what everyone else thinks, but I
> agree now is the time to get these API details agreed.
>
> Cheers,
>
> James.
>
> On 6 Oct 2012, at 16:25, Dave Beech wrote:
>
>> Hi James
>>
>> You know, if it wasn't for the addAllOutput methods I'd completely agree
>> with you. "addAll" is a nice name because it's like the same method on Java
>> collections. But because you've got complementary pairs of input and output
>> methods, it's just that the obsessive-compulsive bit of my brain wants the
>> two to look the same!
>>
>> As for taking a List as parameter, again I agree - 99 times out a 100 I bet
>> a list is what you'll want to pass. But the code is a simple foreach style
>> loop, so my question is why limit it to lists when you could easily pass in
>> a Set or a Queue and it would work fine.
>>
>> I don't feel too strongly about this - just wanted to put it out and get
>> thoughts on it. As we're going for a 1.0.0 release I think it's important we
>> get these little API details agreed now rather than be stuck with something
>> we're not completely happy with.
>>
>> Cheers,
>> Dave
>>
>> On 6 Oct 2012, at 14:46, James Kinley <[email protected]> wrote:
>>
>>> Hi Dave,
>>>
>>> I quite like addAll and withAll and would normally rely on the Javadoc for
>>> the details, but I agree that addInputs and withInputs are more descriptive
>>> names so I'm happy if you want to change them.
>>>
>>> Regarding the input type, what other types of input do you see users
>>> passing in that cannot be handled by List?
>>>
>>> Cheers,
>>>
>>> James.
>>>
>>> On 6 Oct 2012, at 13:09, Dave Beech wrote:
>>>
>>>> Hi guys,
>>>>
>>>> I'm having a go at resolving MRUNIT-138. I'll get a patch out for
>>>> review before commit since I will be breaking backwards-compatibility.
>>>>
>>>> One thing I'd like your opinion on in the meantime. I'm not completely
>>>> happy with the names of the multiple input/output methods added in
>>>> MRUNIT-64. I think they're a little inconsistent with each other and
>>>> in the case of the input ones (withAll, addAll) - not very
>>>> descriptive. I'd like to rename these (not a compatibility issue since
>>>> they aren't yet included in a release version).
>>>>
>>>> What do you think?
>>>>
>>>> withAll -> rename to (a) withInputs OR (b) withAllInput
>>>> addAll -> rename to (a) addInputs OR (b) addAllInput
>>>>
>>>> Obviously if you think (a) is best, I'd rename the withAllOutput
>>>> methods to withOutputs to match.
>>>>
>>>> Also - should the input type of these methods be changed from List to
>>>> Collection (or Iterable maybe), to make it more flexible as to what
>>>> you can pass in?
>>>>
>>>> Thanks,
>>>> Dave
>>>
>
--
Apache MRUnit - Unit testing MapReduce - http://incubator.apache.org/mrunit/
