[JAVA-640] Signature change for aggregate helper method Created: 10/Sep/12  Updated: 14/Feb/14  Resolved: 03/Oct/13

Status: Closed
Project: Java Driver
Component/s: API
Affects Version/s: 2.9.0, 2.9.1
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: David M. Carr Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates JAVA-860 Allow pipeline arguments to the aggre... Closed

 Description   

Any chance you could consider changing the signature of the aggregate
helper method to a single vararg argument, or adding an overloaded
signature that takes List<DBObject>? I assume the reason that the
first op was modelled as a separate argument was to make it clear to
users that at least one operation was required. I actually think
that's adequately clear as just a vararg argument, and having the
first argument separate makes it more complex when you already have
the pipeline as a list or array.

That is, from:

public AggregationOutput aggregate( DBObject firstOp, DBObject ... additionalOps)

to:

public AggregationOutput aggregate( DBObject ... ops)

or add:

public AggregationOutput aggregate( List<DBObject> ops)

The particular case I'm looking at is the run method in the class
below. I'm currently using a workaround that splits the first op and
the remainder before passing them to the helper method.

https://bitbucket.org/davidmc24/mongo-java-aggregate/src/6e0e93bfe2a4/src/main/java/org/bitbucket/davidm24/mongodb/aggregate/AggregateBuilder.java



 Comments   
Comment by Justin Lee [ 03/Oct/13 ]

duplicate of JAVA-860 which resolves this issue

Comment by Jeffrey Yemin [ 03/Jul/13 ]

Especially as it turns out to be perfectly legal to have an empty pipeline, but yes, it's binary-incompatible. We should consider this for 3.0.

Comment by David M. Carr [ 02/Jul/13 ]

I think that the signature that Trisha proposed would be the ideal. Unfortunately, I don't think it's compatible with retaining the existing signature, as it would result in a compile-time "reference to aggregate is ambiguous" error, and I suspect that removing the old signature would be binary incompatible, as Jeff mentioned.

Comment by Trisha Gee [ 02/Jul/13 ]

I see what the issue is. I'm not totally sold on having two different ways of passing an ordered set of operations in (i.e. varargs and a List) when they're more or less the same thing, but I see the issue you're having.

I think having an additional varargs method:

public AggregationOutput aggregate(DBObject ... ops)

is more consistent. It would have to check that the size is at least 1, but that doesn't seem too terrible.

Comment by Vincent Cantin [ 17/Apr/13 ]

Me too I wish to change the signature, it makes it more convenient when you build the pipeline dynamically.
I added the function which takes a List as a parameter: https://github.com/mongodb/mongo-java-driver/pull/110

Comment by David M. Carr [ 12/Sep/12 ]

Sounds reasonable. As I said, I have a workaround, so this isn't particularly urgent.

Comment by Jeffrey Yemin [ 12/Sep/12 ]

The signature change would not be binary compatible, so that's out. The overloaded method with List is a possibility, but I want to see if other users are also unhappy. Your situation is a bit different since you're building a framework.

Generated at Thu Feb 08 08:52:44 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.