Create non object parameters methods of controllers.

[cocytus]

Instead of having to create a parameter object even though a method might just have one object, we can just call the method directly with several arguments.

Instead of: this.someControllerApi.Method({ prop1: value,1 prop2: value2 }) we can do this.someControllerApi.Method_(value1, value2)

[jnwltr]

I fully understand and agree with import * as __model.

On the contrary I am not sure about param-based request invocations. Param objects with defined interfaces define nicely optional vs. required params. Do you think the syntactic sugar you are proposing is really worth? I understand 1-argument methods with required param look a bit simpler, but is it really worth in general?

[cocytus]

Worth it in general: Yes, I think so. It could be reasonable to only generate this for methods with i.e. <= 5 parameters, but in those cases, it's often really useful. I just made a codebase (fairly small, 40~methods or so at this time) use generated controllers, and my guess is 50% of calls used the param-based methods. Having to do svc.method1({param1: val1, param2: val2, param3: val3} as upposed to svc.method1_(val1, val2, val3) is somewhat annoying and takes a little bit longer to read later.

At a previous project we used typelite (or, rather, my fork of it which added method support)-generated angular controllers, and there the default was to create method with several parameters, but add a method with one object where we had > 2 parameters to the method. I think this worked very well.

It should also be noted that this does not affect existing codebases at all, so the "risk" when adding this should be really small. It could also easily be optional.

[cocytus]

Rebased on new rc2 branch, fixed conflicts, won't generate for > 4 parameters

[fazpu]

This is my personal opinion and taste - I rather don't like the param-based request change. It adds unnecessary complexity to the code with no significant benefit.

[cocytus]

@fazpu Why do you think generated methods are special? I doubt you usually have only single-parameter methods in your code and then going around doing method({param1: value1, param..[...]); ? I don't understand why this should be the case for generated methods..

[jnwltr]

I probably perceive somewhat annoying a bit differently, I like simple language (which this duplicity in how to invoke the request does not bring - imagine 2 programmers having different preference in what approach to choose -> mixed patterns in the code base).

Anyway, if you really want to have this, let's

• put it under option (e.g. --params=[objects/args]),
• do the same thing for all methods (no .length <= X condition),
• sort args-styled params by required (they go first and optional params default to undefined).

One more plea, please, rebase and make the new PR to rc/2, this rc2 is obsolete, and I will delete it once you move the PR.

What do you think?

Thanks.

[fazpu]

I feel it might bring confusion to the project. The potential explanation of "if the generated method has 4 parameters or less, you can use syntax A, otherwise you must use syntax B" does not seem right to me.

It would be possible to get rid of the "4 parameters or less" for consistency reasons, but then the generated methods with arguments could be potentially very long and would cause the code being far less readable.

I would stick with one default way how the methods should be generated.

[cocytus]

@jnwltr Yes, this adds two ways of doing a method call - in that sense in it is not simpler. But due to the somewhat awkward way you would have to call methods with just one parameter without this functionality, I do believe it's worth it. As I wrote to @fazpu - why are generated methods special? Wrapping parameters to methods which require more than one actual parameter in a single object is somewhat uncommon.

I will implement and fix your suggestions.

[jnwltr]

@cocytus I do like simplicity. But in this case, object-based params are much more flexible and error-immune (which is btw the main design goal of this generator). In fact, api calls are special. Why?

I have seen many projects, where backend is a living organism, updated on daily basis. You want to regenerate the api layer with same frequency and catch as much changes automatically as possible.

(A) Security Passing parameters as unnamed values would not cach that many changes. Let's imagine the following scenario:

Api v1: domain.com/api/get-order-item/{orderId}/{itemPosition} Api v2: domain.com/api/get-order-item/{orderId}/{itemId}

Both itemPosition and itemId are numbers, but have totally different semantics. Now compare the invocations:

this.controllerService.getOrderItem({orderId: 3, itemPosition: 10}) // error in attribute

vs.

this.controllerService.getOrderItem(3, 10) // no problem

(B) Flexibility Imagine a complex e.g. sorting/paging/filtering model with many attributes. Different places in the app utilize just a few features. So the param object would look like this:

{
  sorting?: string;
  filtering?: string;
  pageSize?: number;
  pageOffset?: number;
  customAggregation?: VeryComplexType;
}

and the invocations for say pure sorting and customAggregation utilization:

this.controllerService.method({sorting: 'column1'});
this.controllerService.method({customAggregation});

vs.

this.controllerService.method('column1'); // let's assume we have defaults
this.controllerService.method(undefined, undefined, undefined, undefined, customAggregation);

So, after reconsidering a bit, I have to confess I do not like you proposal, since it slightly improves some use cases, but significantly worsens others, and introduces complexity to the code base.

Please, reconsider, I believe you'll understand and agree with my arguments.

[cocytus]

@jnwltr Your points are valid, and the arguments have both contributed to the increased support of named parameters in modern programming languages (as opposed to positional). It should be noted that B) is not really a concern if we limit ourselves to methods with only a few parameters, which was my original intention.

However - this is not really unique to generated API client code. This applies to all code. If you change the order of parameters of the same type in any project without telling the consumers of the code you're creating you will have a bad day. Now, one could make the argument that in dynamic languages like javascript you should always pass around objects to single parameter methods across module (or something) boundaries, but I don't think that's very readable, and it is not commonly done.

Anyway, It's your project, and we can just use the npm package as-is with quite a few getThingById({id: id}); calls, or automatically use the tmp/mergetmp branch on my fork (which we're currently doing). I'll confer with my colleagues. I stillthink adding this PR is a good idea after considering pro/con arguments.

Or, another solution: For single parameter methods, skip the wrapper object. That would reduce the annoyance in our project to almost nothing.

[jnwltr]

Thanks to your comments. Please, do not take it as somebody's project. I believe common decisions of the users of different experience and knowledge will lead to best results.

I have used this generator on several projects already and know, that the more sound feedback (with minimum of false positives and negatives) you get after re-generation, the better. Objects betform better than params, here, imo.

After all, I think we are discussing a very cosmetic nuance.

Getting used to .getById({id: 123}) instead of .getById(123) was not that hard for me and calling it annoyance sounds too strong to me. But I fully understand you might have different (still changable) opinion.

Regarding exceptions:

• <= 4 is artificial. Method crossing that boundary in api version upgrade would disappear,
• == 1 is the only one that may have reason from my point of view. If you feel it necessary, let's do it,
• default undefined for optional param has to be set.

[cocytus]

OK, I updated the code so we only generate for single parameter methods.

[cocytus]

Btw, default undefined has to be set - what do you mean? you can't have method(p1?: string = undefined), as p1? means parameter is optional And it defaults to undefined.

[cocytus]

@fazpu Good points, thank you. Test added, refactored block to method.

[cocytus]

@jnwltr Fixed.

[jnwltr]

Released as 2.0.0-rc.6. Thanks, guys.

[cocytus]

Thank you =)