nacx commented on this pull request.
Thanks, @trevorflanagan! Looks great. Just one comment about code reuse and I
think it's good to merge!
> + this.queryParameters.put("datacenterId", checkNotNull(datacenterId,
> "datacenterId"));
+ }
+ return this;
+ }
+
+ public DatacenterIdListFilters paginationOptions(final PaginationOptions
paginationOptions) {
+ if (paginationOptions.orderBy() != null) {
+ this.queryParameters.put("orderBy", paginationOptions.orderBy());
+ }
+ if (paginationOptions.pageNumber() != null) {
+ this.queryParameters.put("pageNumber",
paginationOptions.pageNumber());
+ }
+ if (paginationOptions.pageSize() != null) {
+ this.queryParameters.put("pageSize", paginationOptions.pageSize());
+ }
+ return this;
Just:
```java
public DatacenterIdListFilters paginationOptions(final PaginationOptions
paginationOptions) {
this.queryParameters.putAll(paginationOptions.buildQueryParameters());
return this;
}
```
To avoid duplicating logic.
> + * limitations under the License.
+ */
+package org.jclouds.dimensiondata.cloudcontrol.options;
+
+import org.jclouds.http.options.BaseHttpRequestOptions;
+
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class DatacenterIdListFilters extends BaseHttpRequestOptions {
+
+ private DatacenterIdListFilters() {
+ }
+
+ private DatacenterIdListFilters datacenterIds(final Set<String>
datacenterIds) {
If the common use case will be that there is just one ID to pass, consider
providing a varargs version of the method too, to make it cleaner and easier to
consume.
> @@ -52,7 +54,8 @@ public void testListOperatingSystems() {
for (Datacenter dc : getDatacenters()) {
datacenterIds.add(dc.id());
}
- FluentIterable<OperatingSystem> operatingSystems =
api().listOperatingSystems(datacenterIds).concat();
+ ImmutableList<OperatingSystem> operatingSystems = api()
+
.listOperatingSystems(DatacenterIdListFilters.Builder.datacenterId(datacenterIds)).toList();
[minor] static importing the `datacenterId` method makes the invocation
cleaner. Not something to be changed, just a comment on how, in general, we use
this kind of option objects in jclouds.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/433#pullrequestreview-113543015
