@marcuslinke my thoughts:

• is* should be used only for boolean primitives, get* or has* for Objects
• primitive -> Object doesn't make sense for stream handling/callback internals (was all this change done with sed?)
• all Nullable return values and fields should be annotated with @CheckForNull
• withSomething() looks redundant in interface and it not so difficult to set explicit values for library consumers
• many places will have null checks for data values if (getSomething() != null && getSomething() { so maybe it makes sense to keep isSomething() methods that will have this null check internally (see BuildImageCmd where i kept backward compatibility for primitive method and used Object for data field

@KostyaSha Thanks for the review! Yes, I initially did search and replace for booleans, so I need to review all of these replacements. I like your suggestions especially that it will retain backward compatibility. However, it will take some time as I'm quite busy with other things at the moment.

@marcuslinke my suggestion will be to start doing it partially. I can start with critical for docker-plugin API commands and PR changes. @marcuslinke btw, backward compatibility will be broken in any release because all methods defined in interface that requires extending fields/methods for every docker API update. So it seems that we should forget about compatibilities in X.Y.

@KostyaSha After thinking about it again I want to discuss these additional get/has/is methods that return primitives (in commands):

• Examining existing command objects isn't the general use case I guess, so handling null cases might be acceptable for the caller
• Returning primitives introduces some kind of default value which might be OK for boolean but what about long/int values (see tail member in log command for example)?

Maybe attach to request only non-null values without checking what is default plus annotate everything with @CheckForNull? If docker has some default values, then it's probably not a docker-java issue to show user what is default.

OK, so lets summarize:

• dropping get/is/has methods that return primitives like boolean, long, int
• by annotating json objects with @JsonInclude(Include.NON_NULL) we can consider that only non-null values are sent with the request
• by annotating appropriate get/has/is methods with @CheckForNull FindBugs can find missing null checks in client code

Do you agree?

Sounds good

My idea about removing withSomething() was to minimise code in library.

Yeah. I will probably remove this also. By the way, according to http://ehc.ac/p/findbugs/bugs/334/ @CheckForNull can be annotated at the interface level. So I will remove this annotation from the implementation ('BuildImageCmdImpl'), OK?

Right, for method return values inheritance should work.

@marcuslinke and if we do refactoring, is it possible to use Lists instead arrays?

@KostyaSha I think we could add additional methods that takes Lists.

Disclaimer: i'm not an expert of api libraries :)
@marcuslinke my main concern about List was to have easier data manipulation. For example in docker i fill create command on 2 levels. First is configuration and then launcher modifies parameters. With array if very inconvenient to do changes, it enforces casting to List, working with it and back setting to array.
So logical idea was to simplify api and place List directly.

According to google this code should work

@JsonProperty("Cmd")
private List<String> cmd;

Then you can simply use some http://commons.apache.org/proper/commons-collections/javadocs/api-3.2.1/org/apache/commons/collections/CollectionUtils.html#isNotEmpty(java.util.Collection) or guava...

I think the main difference between arrays and lists is the aspect of mutablitity. Whereas arrays are immutable, lists in contast are not and allow the manipulation from the outside. In general I dont think this is a good idea as it may result in unexpected behaviour. So switching to lists would enforce to ensure the immutablitity by copying to (when implementing the setter) and copying from (when implementing the getter) the internal list. I would avoid this implementation effort...

Where do you see mutability issue? You can either replace List either manipulate with existed. During exec it will be send to remote.