Java: Make it clearer which operation input parameters are required

tadhgpearson commented 8 years ago

Description

Today, in the DefaultAPI class, one method is generated for each operation, and each of these methods has one method parameter for each operation parameter. The only indication that a field is optional or required is a small text marked in brackets at the end of the @param for each method. Our API users don't usually seem to notice this and thus find it difficult to identify which parameters are required and which are optional

Note - I've identified the Java client here because it's the one most of our clients use. However, for example, the Python client doesn't mark which parameters are required and which are optional anywhere, so I suspect this is an improvement which could be made across many languages.

Swagger-codegen version

2.2.1

Swagger declaration file content or url

https://github.com/amadeus-travel-innovation-sandbox/sandbox-content/blob/master/swagger.yml

Command line used for generation

dateLibrary=joda

Steps to reproduce

Call http://api.sandbox.amadeus.com/swagger-codegen/java See generated DefaultAPI class

Related issues

Potentially related to #2485

Suggest a Fix

There are a number of possible improvements I can suggest, but each of them has their pros and cons

Start each @param documentation element by noting whether the element is required (might not be obvious enough, does not benefit from clear highlighting in Eclipse)
Offer 2 DefaultAPI methods for each operation, if that operation has optional parameters. One method should take only the required parameters - the second should have all parameters (default APIs class could get long)
Use the Optional class to denote optional elements in the input methods (method calls could get verbose)
Require an input object for each method, where the object takes all required parameters in the construct and uses optional or setters for other parameters (big change, might be painful for short methods)

wing328 commented 8 years ago

@tadhgpearson thanks for the feedback. It looks to me it's a limitation with the Javadoc at the moment:

Start each @param documentation element by noting whether the element is required (might not be obvious enough, does not benefit from clear highlighting in Eclipse)

This sounds good to me.

karussell commented 7 years ago

Require an input object for each method, where the object takes all required parameters in the construct and uses optional or setters for other parameters (big change, might be painful for short methods)

This gets a big :+1: from me. Probably also for other languages it is better to have a separate request object (instead of one method with maany parameters), and this request object gets required parameters in the constructor and optional parameters via setters. Otherwise you can end up with lots of null parameters which is very unreadable.

tadhgpearson commented 7 years ago

@karussell - Maybe a good approach to this would be to set a limit at which the object-based approach kicks in. For example, use an object only if the call has more than 3 parameters.

karussell commented 7 years ago

I would favor to always use a request object. The one argument is that the POST API is already doing this and the second stronger argument is that if you add optional parameters you do not have to break the API with a request object. Furthermore in Java it is more descriptive and not that complicated. See this example where I set an optional "point" parameter:

response = someApi.someEndpoint(new SomeRequestObject(requiredParam1, requiredParam2).setPoint(point));

tadhgpearson commented 7 years ago

Right, and don't get me wrong, I think it's a fine approach most of the time, except for those very common argument APIs, like

someCRUDApi.getRecord(new RecordID("recordID");
someSearchAPI.search(new SearchQueryRequest("Query"));

in which case it just becomes a verbose string wrapper.

swagger-api / swagger-codegen